move LayoutSpecification types to DynamoCore with type forward#30
Merged
mjkkirschner merged 1 commit intoSHKnudsen:Node-Markdown-generator-toolfrom Aug 16, 2021
Conversation
| using System.Text; | ||
| using System.Text.RegularExpressions; | ||
| using Dynamo.Wpf.Interfaces; | ||
| using Dynamo.Interfaces; |
There was a problem hiding this comment.
Does the code not compile any longer without these additional usings or are these required at runtime. Even though the type forwarding includes the complete namespace, is this still required?
| <Project>{51bb6014-43f7-4f31-b8d3-e3c37ebedaf4}</Project> | ||
| <Name>DynamoCoreWpf</Name> | ||
| <Private>False</Private> | ||
| </ProjectReference> |
SHKnudsen
approved these changes
Aug 12, 2021
Collaborator
Author
|
So the code won’t compile without these usings - because I changed the namespace the type belonged to. But it’s my understanding from the docs on the type forward attribute that previously compiled clients using these types will be able to find them without issue and won’t need to recompile. For clients that do recompile they’ll need to do what I did here and add the usings.
Or - I could keep the namespace the same to avoid that I believe, do you have a preference?
MK
… On Aug 11, 2021, at 5:08 PM, aparajit-pratap ***@***.***> wrote:
@aparajit-pratap commented on this pull request.
In src/Tools/NodeDocumentationMarkdownGenerator/MarkdownHandler.cs:
> @@ -6,7 +6,7 @@
using System.Reflection;
using System.Text;
using System.Text.RegularExpressions;
-using Dynamo.Wpf.Interfaces;
+using Dynamo.Interfaces;
Does the code not compile any longer without these additional usings or are these required at runtime. Even though the type forwarding includes the complete namespace, is this still required?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
|
@mjkkirschner no, this is fine, LGTM! |
aparajit-pratap
approved these changes
Aug 16, 2021
SHKnudsen
added a commit
that referenced
this pull request
Aug 26, 2021
* initial commit * Use CLRDLLModule * Refactoring * add hosts flag and other additions to frompackage * 🧹🧹 * Update Program.cs * clean up + handle overloads * 🧹 * Update AssemblyHandler.cs * too many things 🤯 * Save images instead of embedding * comment updates * add support for .ds files * comments + move to tools folder * add reference to RestSharp - need to create PackageUploadRequestBody * Update FromPackageFolderCommand.cs * 🧹 * Remove logger * remove unused using * Update FromDirectoryOptions.cs * Update AssemblyHandler.cs * Change project type + add test project * remove old files * use flag to specify ReflectionContext * few comment updates * Update DocumentationBrowserViewExtensionTests.cs * ds file fixes * fix add isReflection flag FFIMethodAtribute * First pass at comments * Update MarkdownHandler.cs * 2nd pass on comments * Update MarkdownHandler.cs * help text updates * update licence * comments * add reflection context object type check to CLRDLLModule * Use IconName to get the type FullName from a NodeModelSearchElement + other clean ups * get baseType with overloads * Load dynamo dlls from main bin folder + fix overload lookup * Update DocumentationBrowserViewExtensionTests.cs * fix issue with protogeometry format load error put clr into reflection mode when importing DS files fix classname for generated ds docs * remove unused * remove test project * comment cleanups * merge license (#31) Co-authored-by: michael kirschner <[email protected]> * move LayoutSpecification types to DynamoCore with type forward (#30) Co-authored-by: michael kirschner <[email protected]> * add new flag -g to compress gifs add gif compression by quantization to 32k colors in debug leave console open until readline for easy log reading * typo * try to resolve merge conflicts with dynamo master (#33) * Dyn 3817 exit tour window popup (DynamoDS#11918) * DYN-3817-ExitTourWindow Popup I added the UI for the Exit Tour Window, but still having some issues in the layout * DYN-3817-ExitTourWindow-Popup I created the view for the Exit Tour Window and also the functionality of showing the ExitTour ( this is shown only when one of the steps is closed so the tour is not finished - this doesn't apply for the Survey popup). A resource string was added and is related to the ExitTour content so it can be localized later. * DYN-3817-ExitTourWindow-Popup Removing code not needed in DynamoView.xaml.cs * DYN-3817-ExitTourWindow-Popup Code Review The ExitTourWindow was renamed to RealTimeInfoWindow. I reorganized the RealTimeInfoWindow so doesn't depend of a Step (then it can be used in a more generic way). I created the properties Width, Height and TextContent inside the RealTimeInfoWindow so they can be set up from outside. The TextContent property contains the text that will be shown in the popup and it can be updated on runtime. * DYN-3817-ExitTourWindow-Popup Code Issues Fixing issues produced after merging the latest changes in master branch * DYN-3817-ExitTourWindow-Popup Code Review 2 The offset hardcoded values were changed to const double, also I reorganized the way the RealTimeInfoWindow object is created. Also I added some comments in the xaml file * Dyn-3640 - Recover execution after clearing cyclic dependency from graph (DynamoDS#11896) * cleanup * minor cleanup * clear cycles from dependency graph nodes for graph runs * cleanup * update failing test * clear node warnings for cleared cycles * do not include cyclic graph node as a redefined node of another graph node * add unit tests * fix one more case * review comments * add logging to intermittent pkgloader tests (DynamoDS#11925) * update tests * update xml summary add some logging to failing tests Co-authored-by: michael kirschner <[email protected]> * pkg load test log typo (DynamoDS#11926) * update tests * update xml summary add some logging to failing tests * typo Co-authored-by: michael kirschner <[email protected]> * Update UI (DynamoDS#11932) * Update searchMethod invocation in LibraryMSWebBrowserExtension (DynamoDS#11931) * Update searchMethod invocation Update searchMethod invocation to use correct number of parameters * Update SearchDictionary.cs * Add adsk fonts to extern folder with license terms. (DynamoDS#11890) * small adjustment * align titles * add names to buttons for ui automation tests * missing button name * new font folder for use in Dynamo only * Update readme.md * add font links to about box Co-authored-by: michael kirschner <[email protected]> * check for null (DynamoDS#11930) * Prevent port measure crash (DynamoDS#11935) * CRASH: Delete node with right-click at the same time (DynamoDS#11927) * null checks * Add unit test * Fix For Loading Json File (DynamoDS#11934) * Fix For Loading Json File During the testing of the task https://jira.autodesk.com/browse/DYN-3817 Aabishkar reported that when trying to open the "Get Started" tour a message appear saying Could not find a part of the path 'C:\Windows\system32\UI\GuidedTour\dynamo_guides.json' Then I modified the code that is loading the json,file, in the past was a relative path and now we are using an absolute path. * Fix For Loading Json File Instead to have the json file path in the GuidesManager constructor I created two public properties that will contains the assembly path execution and the JSON files guides path, so we can verify in the unit testing that this paths exists and if the Guide will be created correctly. Also I added a unit test that will validate that the json guides file exists and that the Guides can be created. * Handle crash due to save path being too long Co-authored-by: Roberto T <[email protected]> Co-authored-by: aparajit-pratap <[email protected]> Co-authored-by: michael kirschner <[email protected]> Co-authored-by: Aaron (Qilong) <[email protected]> Co-authored-by: Ashish Aggarwal <[email protected]> * revert changes to clrdllmodule * this builds * remove reflection extensions * add dyf folder exists check * resolve all reference paths remove metadata references * comments * add image magick to license files * add readme * add known issues section * more readme tweaks * add verbose mode and some logging when scanning dlls add console colors for dictionary related errors update readme * add findings to known issues * review comments move ilibraryviewcustomization as well to dynamocore * review comments Co-authored-by: SHKnudsen <[email protected]> Co-authored-by: michael kirschner <[email protected]> Co-authored-by: Roberto T <[email protected]> Co-authored-by: aparajit-pratap <[email protected]> Co-authored-by: Aaron (Qilong) <[email protected]> Co-authored-by: Ashish Aggarwal <[email protected]>
SHKnudsen
added a commit
that referenced
this pull request
Sep 1, 2021
* initial commit * Use CLRDLLModule * Refactoring * add hosts flag and other additions to frompackage * 🧹🧹 * Update Program.cs * clean up + handle overloads * 🧹 * Update AssemblyHandler.cs * too many things 🤯 * Save images instead of embedding * comment updates * add support for .ds files * comments + move to tools folder * add reference to RestSharp - need to create PackageUploadRequestBody * Update FromPackageFolderCommand.cs * 🧹 * Remove logger * remove unused using * Update FromDirectoryOptions.cs * Update AssemblyHandler.cs * Change project type + add test project * remove old files * use flag to specify ReflectionContext * few comment updates * Update DocumentationBrowserViewExtensionTests.cs * ds file fixes * fix add isReflection flag FFIMethodAtribute * First pass at comments * Update MarkdownHandler.cs * 2nd pass on comments * Update MarkdownHandler.cs * help text updates * update licence * comments * add reflection context object type check to CLRDLLModule * Use IconName to get the type FullName from a NodeModelSearchElement + other clean ups * get baseType with overloads * Load dynamo dlls from main bin folder + fix overload lookup * Update DocumentationBrowserViewExtensionTests.cs * fix issue with protogeometry format load error put clr into reflection mode when importing DS files fix classname for generated ds docs * remove unused * remove test project * unit tests first commit * Update Program.cs * comment cleanups * merge license (#31) Co-authored-by: michael kirschner <[email protected]> * move LayoutSpecification types to DynamoCore with type forward (#30) Co-authored-by: michael kirschner <[email protected]> * add new flag -g to compress gifs add gif compression by quantization to 32k colors in debug leave console open until readline for easy log reading * typo * try to resolve merge conflicts with dynamo master (#33) * Dyn 3817 exit tour window popup (DynamoDS#11918) * DYN-3817-ExitTourWindow Popup I added the UI for the Exit Tour Window, but still having some issues in the layout * DYN-3817-ExitTourWindow-Popup I created the view for the Exit Tour Window and also the functionality of showing the ExitTour ( this is shown only when one of the steps is closed so the tour is not finished - this doesn't apply for the Survey popup). A resource string was added and is related to the ExitTour content so it can be localized later. * DYN-3817-ExitTourWindow-Popup Removing code not needed in DynamoView.xaml.cs * DYN-3817-ExitTourWindow-Popup Code Review The ExitTourWindow was renamed to RealTimeInfoWindow. I reorganized the RealTimeInfoWindow so doesn't depend of a Step (then it can be used in a more generic way). I created the properties Width, Height and TextContent inside the RealTimeInfoWindow so they can be set up from outside. The TextContent property contains the text that will be shown in the popup and it can be updated on runtime. * DYN-3817-ExitTourWindow-Popup Code Issues Fixing issues produced after merging the latest changes in master branch * DYN-3817-ExitTourWindow-Popup Code Review 2 The offset hardcoded values were changed to const double, also I reorganized the way the RealTimeInfoWindow object is created. Also I added some comments in the xaml file * Dyn-3640 - Recover execution after clearing cyclic dependency from graph (DynamoDS#11896) * cleanup * minor cleanup * clear cycles from dependency graph nodes for graph runs * cleanup * update failing test * clear node warnings for cleared cycles * do not include cyclic graph node as a redefined node of another graph node * add unit tests * fix one more case * review comments * add logging to intermittent pkgloader tests (DynamoDS#11925) * update tests * update xml summary add some logging to failing tests Co-authored-by: michael kirschner <[email protected]> * pkg load test log typo (DynamoDS#11926) * update tests * update xml summary add some logging to failing tests * typo Co-authored-by: michael kirschner <[email protected]> * Update UI (DynamoDS#11932) * Update searchMethod invocation in LibraryMSWebBrowserExtension (DynamoDS#11931) * Update searchMethod invocation Update searchMethod invocation to use correct number of parameters * Update SearchDictionary.cs * Add adsk fonts to extern folder with license terms. (DynamoDS#11890) * small adjustment * align titles * add names to buttons for ui automation tests * missing button name * new font folder for use in Dynamo only * Update readme.md * add font links to about box Co-authored-by: michael kirschner <[email protected]> * check for null (DynamoDS#11930) * Prevent port measure crash (DynamoDS#11935) * CRASH: Delete node with right-click at the same time (DynamoDS#11927) * null checks * Add unit test * Fix For Loading Json File (DynamoDS#11934) * Fix For Loading Json File During the testing of the task https://jira.autodesk.com/browse/DYN-3817 Aabishkar reported that when trying to open the "Get Started" tour a message appear saying Could not find a part of the path 'C:\Windows\system32\UI\GuidedTour\dynamo_guides.json' Then I modified the code that is loading the json,file, in the past was a relative path and now we are using an absolute path. * Fix For Loading Json File Instead to have the json file path in the GuidesManager constructor I created two public properties that will contains the assembly path execution and the JSON files guides path, so we can verify in the unit testing that this paths exists and if the Guide will be created correctly. Also I added a unit test that will validate that the json guides file exists and that the Guides can be created. * Handle crash due to save path being too long Co-authored-by: Roberto T <[email protected]> Co-authored-by: aparajit-pratap <[email protected]> Co-authored-by: michael kirschner <[email protected]> Co-authored-by: Aaron (Qilong) <[email protected]> Co-authored-by: Ashish Aggarwal <[email protected]> * revert changes to clrdllmodule * this builds * remove reflection extensions * Update NodeConstructionTests.cs * add dyf folder exists check * update tests * Update NodeDocumentationMarkdownGeneratorTests.csproj * clean up * Update NodeConstructionTests.cs * resolve all reference paths remove metadata references * comments * add image magick to license files * change package in test * add readme * add known issues section * more readme tweaks * add verbose mode and some logging when scanning dlls add console colors for dictionary related errors update readme * add findings to known issues * merge fix * review comments move ilibraryviewcustomization as well to dynamocore * fix bug if package is missing bin folder fix bug naming custom node .md files with only category name add test dictionary and extract layout spec remove if node and add refactored if node to test files tests should resolve assemblies from tools bin folder and look for other dlls in core dynam bin output tests to bin folder, but not tool * set function type correcty for global functions set classname on generated .md files correctly on global functions add test for ds files * review comments * add image compression tests fix gif compression if resulting gif is larger than original. * add ref flag test ref flag should be optional * add tests for loading from fallback paths * generate more docs for assemblies in nodes folder * revert version * use mslib assembly instead of libview * try to cleanup hanging dialog from linter test Co-authored-by: SHKnudsen <[email protected]> Co-authored-by: michael kirschner <[email protected]> Co-authored-by: Roberto T <[email protected]> Co-authored-by: aparajit-pratap <[email protected]> Co-authored-by: Aaron (Qilong) <[email protected]> Co-authored-by: Ashish Aggarwal <[email protected]>
SHKnudsen
added a commit
that referenced
this pull request
Sep 1, 2021
* initial commit * Use CLRDLLModule * Refactoring * add hosts flag and other additions to frompackage * 🧹🧹 * Update Program.cs * clean up + handle overloads * 🧹 * Update AssemblyHandler.cs * too many things 🤯 * Save images instead of embedding * comment updates * add support for .ds files * comments + move to tools folder * add reference to RestSharp - need to create PackageUploadRequestBody * Update FromPackageFolderCommand.cs * 🧹 * Remove logger * remove unused using * Update FromDirectoryOptions.cs * Update AssemblyHandler.cs * Change project type + add test project * remove old files * use flag to specify ReflectionContext * few comment updates * Update DocumentationBrowserViewExtensionTests.cs * ds file fixes * fix add isReflection flag FFIMethodAtribute * First pass at comments * Update MarkdownHandler.cs * 2nd pass on comments * Update MarkdownHandler.cs * help text updates * update licence * comments * add reflection context object type check to CLRDLLModule * Use IconName to get the type FullName from a NodeModelSearchElement + other clean ups * get baseType with overloads * Load dynamo dlls from main bin folder + fix overload lookup * Update DocumentationBrowserViewExtensionTests.cs * fix issue with protogeometry format load error put clr into reflection mode when importing DS files fix classname for generated ds docs * remove unused * remove test project * unit tests first commit * Update Program.cs * comment cleanups * merge license (#31) Co-authored-by: michael kirschner <[email protected]> * move LayoutSpecification types to DynamoCore with type forward (#30) Co-authored-by: michael kirschner <[email protected]> * add new flag -g to compress gifs add gif compression by quantization to 32k colors in debug leave console open until readline for easy log reading * typo * try to resolve merge conflicts with dynamo master (#33) * Dyn 3817 exit tour window popup (DynamoDS#11918) * DYN-3817-ExitTourWindow Popup I added the UI for the Exit Tour Window, but still having some issues in the layout * DYN-3817-ExitTourWindow-Popup I created the view for the Exit Tour Window and also the functionality of showing the ExitTour ( this is shown only when one of the steps is closed so the tour is not finished - this doesn't apply for the Survey popup). A resource string was added and is related to the ExitTour content so it can be localized later. * DYN-3817-ExitTourWindow-Popup Removing code not needed in DynamoView.xaml.cs * DYN-3817-ExitTourWindow-Popup Code Review The ExitTourWindow was renamed to RealTimeInfoWindow. I reorganized the RealTimeInfoWindow so doesn't depend of a Step (then it can be used in a more generic way). I created the properties Width, Height and TextContent inside the RealTimeInfoWindow so they can be set up from outside. The TextContent property contains the text that will be shown in the popup and it can be updated on runtime. * DYN-3817-ExitTourWindow-Popup Code Issues Fixing issues produced after merging the latest changes in master branch * DYN-3817-ExitTourWindow-Popup Code Review 2 The offset hardcoded values were changed to const double, also I reorganized the way the RealTimeInfoWindow object is created. Also I added some comments in the xaml file * Dyn-3640 - Recover execution after clearing cyclic dependency from graph (DynamoDS#11896) * cleanup * minor cleanup * clear cycles from dependency graph nodes for graph runs * cleanup * update failing test * clear node warnings for cleared cycles * do not include cyclic graph node as a redefined node of another graph node * add unit tests * fix one more case * review comments * add logging to intermittent pkgloader tests (DynamoDS#11925) * update tests * update xml summary add some logging to failing tests Co-authored-by: michael kirschner <[email protected]> * pkg load test log typo (DynamoDS#11926) * update tests * update xml summary add some logging to failing tests * typo Co-authored-by: michael kirschner <[email protected]> * Update UI (DynamoDS#11932) * Update searchMethod invocation in LibraryMSWebBrowserExtension (DynamoDS#11931) * Update searchMethod invocation Update searchMethod invocation to use correct number of parameters * Update SearchDictionary.cs * Add adsk fonts to extern folder with license terms. (DynamoDS#11890) * small adjustment * align titles * add names to buttons for ui automation tests * missing button name * new font folder for use in Dynamo only * Update readme.md * add font links to about box Co-authored-by: michael kirschner <[email protected]> * check for null (DynamoDS#11930) * Prevent port measure crash (DynamoDS#11935) * CRASH: Delete node with right-click at the same time (DynamoDS#11927) * null checks * Add unit test * Fix For Loading Json File (DynamoDS#11934) * Fix For Loading Json File During the testing of the task https://jira.autodesk.com/browse/DYN-3817 Aabishkar reported that when trying to open the "Get Started" tour a message appear saying Could not find a part of the path 'C:\Windows\system32\UI\GuidedTour\dynamo_guides.json' Then I modified the code that is loading the json,file, in the past was a relative path and now we are using an absolute path. * Fix For Loading Json File Instead to have the json file path in the GuidesManager constructor I created two public properties that will contains the assembly path execution and the JSON files guides path, so we can verify in the unit testing that this paths exists and if the Guide will be created correctly. Also I added a unit test that will validate that the json guides file exists and that the Guides can be created. * Handle crash due to save path being too long Co-authored-by: Roberto T <[email protected]> Co-authored-by: aparajit-pratap <[email protected]> Co-authored-by: michael kirschner <[email protected]> Co-authored-by: Aaron (Qilong) <[email protected]> Co-authored-by: Ashish Aggarwal <[email protected]> * revert changes to clrdllmodule * this builds * remove reflection extensions * Update NodeConstructionTests.cs * add dyf folder exists check * update tests * Update NodeDocumentationMarkdownGeneratorTests.csproj * clean up * Update NodeConstructionTests.cs * resolve all reference paths remove metadata references * comments * add image magick to license files * change package in test * add readme * add known issues section * more readme tweaks * add verbose mode and some logging when scanning dlls add console colors for dictionary related errors update readme * add findings to known issues * merge fix * review comments move ilibraryviewcustomization as well to dynamocore * fix bug if package is missing bin folder fix bug naming custom node .md files with only category name add test dictionary and extract layout spec remove if node and add refactored if node to test files tests should resolve assemblies from tools bin folder and look for other dlls in core dynam bin output tests to bin folder, but not tool * set function type correcty for global functions set classname on generated .md files correctly on global functions add test for ds files * review comments * add image compression tests fix gif compression if resulting gif is larger than original. * add ref flag test ref flag should be optional * add tests for loading from fallback paths * generate more docs for assemblies in nodes folder * revert version * use mslib assembly instead of libview * try to cleanup hanging dialog from linter test * fix typo and test Co-authored-by: SHKnudsen <[email protected]> Co-authored-by: michael kirschner <[email protected]> Co-authored-by: Roberto T <[email protected]> Co-authored-by: aparajit-pratap <[email protected]> Co-authored-by: Aaron (Qilong) <[email protected]> Co-authored-by: Ashish Aggarwal <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
@SHKnudsen @aparajit-pratap @QilongTang
PTAL -
The purpose of this PR is remove the DynamoCore.Wpf dependency from the NodeMarkdown docs generator CLI.
this PR uses type forwarding to move the 5 layout spec types from dynamoCore to DynamoCoreWPF:
https://docs.microsoft.com/en-us/dotnet/standard/assembly/type-forwarding
Theres no reason they need to be in DynamoCore.Wpf anyway, they have no deps on WPF.
this should allow old callers to continue working without recompiling - for those clients who do try to recompile on a newer Dynamo, all they should need to do is add
Dynamo.Interfacesto their list of using statements.