DYN-4687 - docs browser and docs generator produce .md files with names that are too long.#13588
DYN-4687 - docs browser and docs generator produce .md files with names that are too long.#13588sm6srw merged 15 commits intoDynamoDS:masterfrom
Conversation
…)" This reverts commit 9338ca4.
sm6srw
changed the title
|
@QilongTang @sm6srw I think it's worth consider if this will affect localization in any negative way - I can't remember exactly what the localization process is for these files today. |
This reverts commit b09be40.
I will look into how this works today and go from there. Thanks! |
All the node help docs are located in each language folder these days. They are localized and will be post to other language folder same as other resources. The default doscs are in |
| /// </summary> | ||
| /// <param name="bytes">hash as a byte array</param> | ||
| /// <returns>hash a valid filename string</returns> | ||
| internal static string GetFilenameFromHash(byte[] bytes) |
There was a problem hiding this comment.
is the encoding/decoding logic specific to filenames ? looks pretty generic to me
There was a problem hiding this comment.
What we get back are guaranteed to work as a file name in all file systems. That's why I named it that way (and used base32).
| var path = Path.GetDirectoryName(file); | ||
| var newFile = Path.Combine(path, shortName + ".md"); | ||
| File.WriteAllText(newFile, $"<!--- {baseName} --->\n" + content); | ||
| File.Delete(file); |
There was a problem hiding this comment.
I wonder if we need to expect permission issues? only devs will run this command ?
There was a problem hiding this comment.
We will get exceptions if that is the case and those are catched and reported at a higher level in the handle function for the rename command. Yes, this is an internal tool. It is not shipped.
src/Tools/NodeDocumentationMarkdownGenerator/Commands/RenameCommand.cs
Outdated
Show resolved
Hide resolved
| private static void RenameFile(string file, string baseName, string shortName) | ||
| { | ||
| var content = File.ReadAllText(file); | ||
| content = content.Replace(baseName, shortName); |
There was a problem hiding this comment.
Are you saving the hashed name in the file contents as well?
There was a problem hiding this comment.
Nop, only the original name. But I could easily do it.
There was a problem hiding this comment.
I meant to ask what is the purpose of line 50 if it's not necessary to save the hashed name in the file?
There was a problem hiding this comment.
That is for replacing the base name of all support files (images and example files etc). Sorry, I misunderstood your question.
What you describe is what I have seen in my testing. I took a look at our latest 2.17 and 2.18 release zips and how this looks like in other locales. I think we are OK. These new renamed files that we introduce (and we will only have a few) should behave as the others. I will merge this and then watch our daily builds. I have no plans right now to get this into 2.17. |
5 participants
Purpose
This pull request does:
Declarations
Check these if you believe they are true
*.resxfilesRelease Notes
(FILL ME IN) Brief description of the fix / enhancement. Mandatory section
Reviewers
(FILL ME IN) Reviewer 1 (If possible, assign the Reviewer for the PR)
(FILL ME IN, optional) Any additional notes to reviewers or testers.
FYIs
(FILL ME IN, Optional) Names of anyone else you wish to be notified of