DYN-4687 - docs browser and docs generator produce .md files with names that are too long. #13588

sm6srw · 2022-12-01T13:35:49Z

Purpose

This pull request does:

add support for fixed length named md files in the fall back folder
add a new rename verb to the generate markdown tool
add tests

Declarations

Check these if you believe they are true

The codebase is in a better state after this PR
Is documented according to the standards
The level of testing this PR includes is appropriate
User facing strings, if any, are extracted into *.resx files
All tests pass using the self-service CI.
Snapshot of UI changes, if any.
Changes to the API follow Semantic Versioning and are documented in the API Changes document.
This PR modifies some build requirements and the readme is updated

Release 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

…4687

.gitignore

…)" This reverts commit 9338ca4.

src/DynamoUtilities/Hash.cs

mjkkirschner · 2022-12-02T18:48:09Z

@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.

sm6srw · 2022-12-02T19:56:16Z

@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.

I will look into how this works today and go from there. Thanks!

QilongTang · 2022-12-03T00:24:21Z

@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.

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 en-US folder as part a copy step during build.

pinzart90 · 2022-12-05T14:17:16Z

src/DynamoUtilities/Hash.cs

+        /// </summary>
+        /// <param name="bytes">hash as a byte array</param>
+        /// <returns>hash a valid filename string</returns>
+        internal static string GetFilenameFromHash(byte[] bytes)


is the encoding/decoding logic specific to filenames ? looks pretty generic to me

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).

pinzart90 · 2022-12-05T14:22:34Z

src/Tools/NodeDocumentationMarkdownGenerator/Commands/RenameCommand.cs

+            var path = Path.GetDirectoryName(file);
+            var newFile = Path.Combine(path, shortName + ".md");
+            File.WriteAllText(newFile, $"<!--- {baseName} --->\n" + content);
+            File.Delete(file);


I wonder if we need to expect permission issues? only devs will run this command ?

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/DynamoUtilities/Hash.cs

src/Tools/NodeDocumentationMarkdownGenerator/Commands/RenameCommand.cs

aparajit-pratap · 2022-12-05T14:23:18Z

src/Tools/NodeDocumentationMarkdownGenerator/Commands/RenameCommand.cs

+        private static void RenameFile(string file, string baseName, string shortName)
+        {
+            var content = File.ReadAllText(file);
+            content = content.Replace(baseName, shortName);


Are you saving the hashed name in the file contents as well?

Nop, only the original name. But I could easily do it.

I meant to ask what is the purpose of line 50 if it's not necessary to save the hashed name in the file?

That is for replacing the base name of all support files (images and example files etc). Sorry, I misunderstood your question.

src/Tools/NodeDocumentationMarkdownGenerator/Commands/RenameCommand.cs

sm6srw · 2022-12-05T14:33:26Z

@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.

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 en-US folder as part a copy step during build.

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.

sm6srw added 2 commits December 1, 2022 08:30

Initial implementation

20d2f84

Merge branch 'master' of https://github.com/DynamoDS/Dynamo into DYN-…

0e0ffad

…4687

sm6srw added the WIP label Dec 1, 2022

Remove files added by mistake.

380bc8f

mjkkirschner reviewed Dec 1, 2022

View reviewed changes

.gitignore Outdated Show resolved Hide resolved

sm6srw added 8 commits December 1, 2022 14:31

Revert .gitignore change

7dd287e

Update help text

5663022

Add the original base name as a comment in the renamed MD file

a06ce38

Revert "temp fix for builds, remove tsplines nodes docs (DynamoDS#12693…

347c25a

…)" This reverts commit 9338ca4.

Rename offending files

b09be40

Add hash test

866bab9

Add tests for markdown generator tool

574c9d8

Update Readme

3d312ce

sm6srw changed the title ~~WIP: DYN-4687 - docs browser and docs generator produce .md files with names that are too long.~~ DYN-4687 - docs browser and docs generator produce .md files with names that are too long. Dec 2, 2022

sm6srw removed the WIP label Dec 2, 2022

Add comments

ebb2032

mjkkirschner reviewed Dec 2, 2022

View reviewed changes

src/DynamoUtilities/Hash.cs Outdated Show resolved Hide resolved

mjkkirschner approved these changes Dec 2, 2022

View reviewed changes

sm6srw added 2 commits December 2, 2022 14:33

Revert "Rename offending files"

76477a1

This reverts commit b09be40.

Use SHA256/Base32 instead of SHA1

1ba1c6c

pinzart90 reviewed Dec 5, 2022

View reviewed changes

aparajit-pratap reviewed Dec 5, 2022

View reviewed changes

Address review comments

c69da4a

aparajit-pratap approved these changes Dec 5, 2022

View reviewed changes

sm6srw merged commit 83161e3 into DynamoDS:master Dec 6, 2022

sm6srw deleted the DYN-4687 branch December 6, 2022 17:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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. #13588

sm6srw commented Dec 1, 2022 •

edited

Loading

mjkkirschner commented Dec 2, 2022

sm6srw commented Dec 2, 2022

QilongTang commented Dec 3, 2022

pinzart90 Dec 5, 2022

sm6srw Dec 5, 2022 •

edited

Loading

pinzart90 Dec 5, 2022

sm6srw Dec 5, 2022 •

edited

Loading

aparajit-pratap Dec 5, 2022

sm6srw Dec 5, 2022 •

edited

Loading

aparajit-pratap Dec 5, 2022

sm6srw Dec 6, 2022

sm6srw commented Dec 5, 2022 •

edited

Loading

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. #13588

Conversation

sm6srw commented Dec 1, 2022 • edited Loading

Purpose

Declarations

Release Notes

Reviewers

FYIs

mjkkirschner commented Dec 2, 2022

sm6srw commented Dec 2, 2022

QilongTang commented Dec 3, 2022

pinzart90 Dec 5, 2022

Choose a reason for hiding this comment

sm6srw Dec 5, 2022 • edited Loading

Choose a reason for hiding this comment

pinzart90 Dec 5, 2022

Choose a reason for hiding this comment

sm6srw Dec 5, 2022 • edited Loading

Choose a reason for hiding this comment

aparajit-pratap Dec 5, 2022

Choose a reason for hiding this comment

sm6srw Dec 5, 2022 • edited Loading

Choose a reason for hiding this comment

aparajit-pratap Dec 5, 2022

Choose a reason for hiding this comment

sm6srw Dec 6, 2022

Choose a reason for hiding this comment

sm6srw commented Dec 5, 2022 • edited Loading

sm6srw commented Dec 1, 2022 •

edited

Loading

sm6srw Dec 5, 2022 •

edited

Loading

sm6srw Dec 5, 2022 •

edited

Loading

sm6srw Dec 5, 2022 •

edited

Loading

sm6srw commented Dec 5, 2022 •

edited

Loading