Repository navigation
fix(studio-server): a rename rewrites only references that name the renamed path - #4915
Conversation
Edit accuracy: accurate 1216 (base branch 1216), smooth 1123 of thoseThe gate passes. Quarantined, measured but not gated (1)
|
…escaped spellings
jrusso1020
left a comment
There was a problem hiding this comment.
Reviewed at 72d1da82: referenceRewriter, referencePattern, projectPaths/directoryReal, the updateReferences call site in the rename route, and the new tests. Approving.
Is the matching exact?
- Start of a reference:
REFERENCE_STARTrefuses a match after a word character,.,/,\,+or-. Somy-assets/,my+assets/,other/assets/,other\assets\andhttps://cdn/assets/are never taken for a rootassets. The.//..///lead is part of the match, and the lookbehind sits before it, so the regex can't restart inside the lead (other/./assetsandother/../assetsstay put). - End of a folder: it must be followed by a separator, which rules out
assets-backup/,assets2/and bare proseassets. - End of a file:
FILE_ENDrefuses a word character,-or.x, which rules outa.png2,a.png-oldanda.png.bak.?v=2,#x,), a closing quote,srcset's2xand a sentence-ending.are all still taken. - Longer paths that exist: a match inside the name of a real file or folder (
other assets/,a.png&backup.png) is left alone. The index is built afterrenameSync, so the old path's own entries are gone and can't shield it. Linked folders count, throughdirectoryReal, with a cycle guard andisSafePath. - The replacement is a function, so a
$&or$1in the new name is written literally. I checked that.
Tests I ran (vitest at this head):
files.renameReferences.test.tspasses, 16 of 16.- The route tests discriminate. With
files.tsrestored to the base, both "renaming a folder over the route" tests fail. The base wrotebrand-backup/,other brand/,empty brand/and "The brand folder", and rewroteother assets/a.pngthrough the linked folder. - In
files.test.ts, "rejects a stale semantic no-op after a concurrent file write" timed out in my checkout. It times out on the base code too, and it doesn't involve renames.
Nits (not blocking):
- A single
\counts as a separator, so a JSON or JS string escape after a folder's name reads as a path. Renaming the folderassetstobrandturns{"label":"The \"assets\" folder","msg":"Loading assets\nnow"}into"The \"brand\" folder"and"Loading brand\nnow".fonts\treadydoes the same. This is far narrower than the base, which rewrote everyassetsin the file, but it's the one prose case left. One fix is to treat a single backslash as a separator only when it isn't followed byn,t,r,",'oru, or only in.md/.mdxfiles. projectPathswalks folders thatwalkFilesskips (renders,.thumbnails,.transcode-cache). That only costs time on each rename. It doesn't change results, because those paths only ever shield a match. Skipping the same set would keep a project with many render frames quick.- A lead deeper than four levels isn't rewritten:
../../../../../assets/a.pngstays as it is. That's safe, but the limit could be noted in the pattern's comment.
Verdict: APPROVE
Reasoning: The pattern takes a path only when it starts at a reference boundary and ends where the name ends. Longer paths that exist are shielded, and the route tests fail on the old split-and-join. What's left is one residual prose case behind JSON/JS string escapes, and two cost and coverage nits.
— Rames Jusso
What changes
Renaming a file or folder rewrote every occurrence of its old path as a plain substring in project text files. Renaming
assetstobrandalso changedassets-backup/b.pngtobrand-backup/b.pngand the words "the assets folder" in prose.The old path counts as a reference only where it starts and ends at a non-name character, after any
./,../or/lead (kept). A folder is matched by what is under it, never by prose. Because a filename can hold any character a delimiter can (a.png&b.png,other assets/), a match inside the longer path of a file or folder that exists in the project is left alone: the project's real paths settle what text cannot. One function owns it (replaceReferences), used by the rename route.Tests
files.renameReferences.test.ts(new): the pattern on every form a project writes a path, a longer name and prose left alone, and the PATCH route renaming a folder next toassets-backupand prose.files.test.tsandfiles.pathSafety.test.tsstill pass (143 tests together on miga).Also read as the same path
A root-relative lead (
/assets/x), JSON-escaped (assets\/x) and Windows (assets\x) separators are rewritten, and a file's path ends at a delimiter, soa.png@2x.pngis not taken fora.png.Known limits
A bare folder name with no
/after it (for example"assets"as a config value) is no longer rewritten when its folder is renamed; it was indistinguishable from prose.References from a nested file written relative to that file (
../assets/xinscenes/css/style.csswhenscenes/assetsis renamed) are still not resolved: the rewrite has no per-file base, as before this change.