DYN-5791 Fixing Renaming Tool #14812
Conversation
If the md file name contains empty spaces then the img file name will also contain %20 instead of empty space so in this way will be loaded by DocumentationBrowser correctly. So my fix is to remove the %20 when renaming the files and the md file content so matches the expected hashed img name.
|
GIF showing that the renaming tool is working as expected when the file name has empty spaces. |
UI Smoke TestsTest: success. 2 passed, 0 failed. |
|
you should add a test for this change. |
| @@ -61,7 +61,7 @@ private static void RenameFile(string file) | |||
|
|
|||
| private static void RenameFile(string file, string baseName, string shortName) | |||
| { | |||
| var content = File.ReadAllText(file); | |||
| var content = HttpUtility.UrlDecode(File.ReadAllText(file)); | |||
There was a problem hiding this comment.
why have you only modified this overload of RenameFile / Directory - now the others are inconsistent in behavior? Is there a good reason?
There was a problem hiding this comment.
The problem is only happening when is trying to replace the image name inside the .md file with the hashed file name and this RenameFile(string file, string baseName, string shortName) method is doing that task,
e.g. for ByOriginVectors node the next values will be replaced.
baseName = "Autodesk.DesignScript.Geometry.CoordinateSystem.ByOriginVectors(origin, xAxis, yAxis, zAxis)"
shortName = "KLFVJ5DI7APVNODARMSE3JM5C27DG62OXUUQRS3HCDTZYO6IBOZA"
The other RenameFile(string file) function is calling RenameFile(string file, string baseName, string shortName), then I don't understand what you mean by "inconsistent in behavior".
There was a problem hiding this comment.
Thanks for the explanation- that makes sense, so now all replace methods will eventually call down to this one and have the same behavior with decoding?
There was a problem hiding this comment.
if with "now all replace methods" you refer to calling "string.Replace", this line updated just applies to the case of replacing the md file content, there are other calls to "string.Replace" that won't need to use HttpUtility.UrlDecode (like the image below) due that is replacing the file name and file names doesn't have the "%20" char.
There are two test cases for using "NodeDocumentationMarkdownGenerator rename" command, one is when renaming just a file and the other when renaming all the files in a folder. If you have a specific case in mind in which this code could not work, please let me know and I can test it. Thanks
There was a problem hiding this comment.
Sorry, typo, I meant Rename not replace.
RenameFile
RenameDirectory
I want to understand if this tool will now always do this decoding whenever it replaces the content inside a file while renaming no matter what code path is used.
There was a problem hiding this comment.
yes, it will do the decoding whenever replaces the content inside a md file.
Here you can see a flow diagram showing that when using "rename" option in both cases will be decoding, I hope this diagram shows the behavior in a more clear way:
Ok, I will add a test for this case |
Adding unit test that will validate that the files are correctly renamed in a folder and that the md file content is updated correctly.
A unit test that will validate the rename of files inside a folder was added |
UI Smoke TestsTest: success. 2 passed, 0 failed. |
|
any more comment or suggestion in this PR? |
|
hi @RobertGlobant20 Do we need to rerun the updated tool to the md files before removing the duplicates? |
@QilongTang we will need to delete the rename_log.txt file, delete the new files (the hashed name ones) and also get the original md/jpg/dyn files (the original files are copied to the en-US\fallback_docs when the Dynamo solution is rebuild), then I guess the renaming tool was executed automatically by a job, or how did you execute the renaming tool?. For example if before running the tool (without my fix) we had the next files:
After running the tool (without my fix) we will have the next files (the md file content is wrong):
Then if we want to run the tool (WITH my fix), then we need the original files and delete all the KLFVJ5DI7APVNODARMSE3JM5C27DG62OXUUQRS3HCDTZYO6IBOZA* files otherwise will throw exceptions. |
|
HI @RobertGlobant20 I think @sm6srw would know because he developed and ran the tool before, not me. |
Purpose
Renaming tool failing to rename the hashed name inside the md file content when the file name has empty spaces (%20).
If the md file name contains empty spaces then the img file name will also contain %20 instead of empty space so in this way will be loaded by DocumentationBrowser correctly. So my fix is to remove the %20 when renaming the files and the md file content so matches the expected hashed img name.
Declarations
Check these if you believe they are true
*.resxfilesRelease Notes
Renaming tool failing to rename the hashed name inside the md file content when the file name has empty spaces (%20).
Reviewers
@QilongTang
FYIs
@reddyashish