Linter ViewExtension #11634

SHKnudsen · 2021-04-20T11:10:54Z

Purpose

This PR does:

Adds new view extension that:
- Enables users to select between loaded linter extension
- Displays all Node- and GraphRule issues from the selected linter

Linter ViewExtension

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

Reviewers

@saintentropy @nate-peters @BogdanZavu

FYIs

@mjkkirschner

…d filter to rules

src/LintingViewExtension/LinterView.xaml

src/LintingViewExtension/Controls/GraphRuleIssueDto.cs

src/LintingViewExtension/LintingViewExtension.cs

mjkkirschner · 2021-04-20T14:50:54Z

src/LintingViewExtension/LintingViewExtension.csproj

+    <DebugSymbols>true</DebugSymbols>
+    <DebugType>full</DebugType>
+    <Optimize>false</Optimize>
+    <OutputPath>..\..\bin\AnyCPU\Debug\</OutputPath>


I think these projects should probably be setup with the imported output paths and assembly shared info generator, unless you want this assembly to have a different version number purposefully.

@mjkkirschner hm, should be setup with the Assembly Shared info, but not sure i know about the imported output paths where are they coming from?

the output paths should be coming from:
https://github.com/DynamoDS/Dynamo/blob/master/src/Config/CS.props#L7
if you are importing this - I think if you define them again here you'll overwite that import if it appears further up in the csproj file.

Yea i was importing it, not sure how i ended up overwriting it.. Should be fixed now.

src/LintingViewExtension/Controls/IRuleIssue.cs

SHKnudsen · 2021-04-28T11:28:40Z

Made some updates to the UI, FYI @nate-peters

Help link to Documentaiton Browser

src/LintingViewExtension/Docs/LinterViewHelpDoc.html

BogdanZavu · 2021-05-03T13:38:00Z

@SHKnudsen since we merged the linter manager branch let's rebase this one

BogdanZavu · 2021-05-04T14:06:25Z

src/DynamoCore/Linting/LinterManager.cs

@@ -131,7 +131,8 @@ private void OnRuleEvaluated(IRuleEvaluationResult result)
            }
        }

-        private LinterExtensionBase GetLinterExtension(LinterExtensionDescriptor activeLinter)
+
+        internal LinterExtensionBase GetLinterExtension(LinterExtensionDescriptor activeLinter)
        {
            return this.extensionManager.


activeLinter might be null

BogdanZavu · 2021-05-04T14:07:55Z

I would say to account for no active linter descriptor as a valid use case (e.g we are throwing during SerializationConverters::WriteJson on startup ).
This will be the initial state I believe once we go live but also users might decide (most likely temporarily) to not have any linting even if they have a few linters available.

src/LintingViewExtension/LinterView.xaml

BogdanZavu · 2021-05-06T16:09:34Z

@SHKnudsen based on our last discussion I think there only a few things left before we can merge :
have a guid for None descriptor
remove debugger.break statements and TestLinterExtension project
Now or in the PR from AGD-2060 to master, let's limit api exposure until this becomes available as public api (when we refactor our rules in the extension) by making LinterExtensionBase internal and preventing LinterViewExtension from showing up in the UI.
If I missed anything please add

SHKnudsen · 2021-05-06T17:22:21Z

@BogdanZavu is there a mechanism to prevent the extension to show up?

BogdanZavu · 2021-05-06T18:15:52Z

@SHKnudsen I would say to interfere when all the ViewExtensions are being loaded ( in DynamoView.xaml.cs ) and skip our extension from the beginning based on its UniquiId. Cant' think of anything better.

mjkkirschner · 2021-05-06T19:55:06Z

Aaron implemented a way to disable extensions - I can’t remember where the config is loaded from though

…

On May 6, 2021, at 2:16 PM, Bogdan Zavu ***@***.***> wrote: @SHKnudsen I would say to interfere when all the ViewExtensions are being loaded and skip our extension from the beginning based on its UniquiId. Cant' think of anything better. https://github.com/SHKnudsen/Dynamo/blob/c7e3c19e8b7feacecd8b2b1d9159ad7cc4b446af/src/DynamoCoreWpf/Views/Core/DynamoView.xaml.cs#L160 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

SHKnudsen · 2021-05-07T08:09:13Z

@QilongTang would you be able to help with Mikes comment above? 👆

QilongTang · 2021-05-07T09:57:52Z

Aaron implemented a way to disable extensions - I can’t remember where the config is loaded from though
…
On May 6, 2021, at 2:16 PM, Bogdan Zavu @.***> wrote: @SHKnudsen I would say to interfere when all the ViewExtensions are being loaded and skip our extension from the beginning based on its UniquiId. Cant' think of anything better. https://github.com/SHKnudsen/Dynamo/blob/c7e3c19e8b7feacecd8b2b1d9159ad7cc4b446af/src/DynamoCoreWpf/Views/Core/DynamoView.xaml.cs#L160 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

@QilongTang would you be able to help with Mikes comment above? 👆

Please refer to the following view extension manifest file: https://github.com/DynamoDS/Dynamo/blob/master/src/NodeAutoCompleteViewExtension/NodeAutoComplete_ViewExtensionDefinition.xml#L4. If you implement this and include the IsEnabled field, it can act as a switch to turn on loading this view extension or not. If you do not include this field, the view extension will be treated as always loaded. Happy to answer any more question.

SHKnudsen · 2021-05-07T11:19:05Z

Ah, so easy! thanks, @QilongTang .

@BogdanZavu this should be ready now unless there are additional comments?

* Initial commit * Move event subscription to extension base class + add property changed filter to rules * comment updates * Update LinterExtensionBase.cs * Update InputNodesNotAllowedRule.cs * Add Deactivate method to linterExtensionBase + LinterManager tests * comment updates * Initial commit * Add issues count to WorkspaceSaving * add scrollviewer * Dispose LinterManager * Make concrete issues internal * summaries on IRuleIssue * comment updates * Update severity code icon * Serialize linter manager to dyn file * clear ruleEvaluationResults on workspace change * Remove test extension * Show names instead of GUIDs * add help doc * clean up * Handle GraphRuleEvaluationResults nodeIds changes * Update LintingViewExtension.csproj * Update LintingViewExtension.csproj * Update SerializationConverters.cs * Fix failing serialization test * comment updates * revert changes to test file * fix available linters binding * add rules back in test ext * comment updates * remove output path * Update LinterManagerTests.cs * comment updates * hide LinterViewExtension and LinterExtensionBase * Update LinterViewModel.cs

* Linter manager (#11606) * Initial commit * Move event subscription to extension base class + add property changed filter to rules * comment updates * Update LinterExtensionBase.cs * Update InputNodesNotAllowedRule.cs * Add Deactivate method to linterExtensionBase + LinterManager tests * comment updates * Dispose LinterManager * comment updates * Serialize linter manager to dyn file * clear ruleEvaluationResults on workspace change * Remove test extension * Handle GraphRuleEvaluationResults nodeIds changes * Update SerializationConverters.cs * Fix failing serialization test * comment updates * revert changes to test file * Linter ViewExtension (#11634) * Initial commit * Move event subscription to extension base class + add property changed filter to rules * comment updates * Update LinterExtensionBase.cs * Update InputNodesNotAllowedRule.cs * Add Deactivate method to linterExtensionBase + LinterManager tests * comment updates * Initial commit * Add issues count to WorkspaceSaving * add scrollviewer * Dispose LinterManager * Make concrete issues internal * summaries on IRuleIssue * comment updates * Update severity code icon * Serialize linter manager to dyn file * clear ruleEvaluationResults on workspace change * Remove test extension * Show names instead of GUIDs * add help doc * clean up * Handle GraphRuleEvaluationResults nodeIds changes * Update LintingViewExtension.csproj * Update LintingViewExtension.csproj * Update SerializationConverters.cs * Fix failing serialization test * comment updates * revert changes to test file * fix available linters binding * add rules back in test ext * comment updates * remove output path * Update LinterManagerTests.cs * comment updates * hide LinterViewExtension and LinterExtensionBase * Update LinterViewModel.cs * if statements * remove Prism * clean up

* Linter manager (#11606) * Initial commit * Move event subscription to extension base class + add property changed filter to rules * comment updates * Update LinterExtensionBase.cs * Update InputNodesNotAllowedRule.cs * Add Deactivate method to linterExtensionBase + LinterManager tests * comment updates * Dispose LinterManager * comment updates * Serialize linter manager to dyn file * clear ruleEvaluationResults on workspace change * Remove test extension * Handle GraphRuleEvaluationResults nodeIds changes * Update SerializationConverters.cs * Fix failing serialization test * comment updates * revert changes to test file * Linter ViewExtension (#11634) * Initial commit * Move event subscription to extension base class + add property changed filter to rules * comment updates * Update LinterExtensionBase.cs * Update InputNodesNotAllowedRule.cs * Add Deactivate method to linterExtensionBase + LinterManager tests * comment updates * Initial commit * Add issues count to WorkspaceSaving * add scrollviewer * Dispose LinterManager * Make concrete issues internal * summaries on IRuleIssue * comment updates * Update severity code icon * Serialize linter manager to dyn file * clear ruleEvaluationResults on workspace change * Remove test extension * Show names instead of GUIDs * add help doc * clean up * Handle GraphRuleEvaluationResults nodeIds changes * Update LintingViewExtension.csproj * Update LintingViewExtension.csproj * Update SerializationConverters.cs * Fix failing serialization test * comment updates * revert changes to test file * fix available linters binding * add rules back in test ext * comment updates * remove output path * Update LinterManagerTests.cs * comment updates * hide LinterViewExtension and LinterExtensionBase * Update LinterViewModel.cs * remove unused test files * Fix pr comments (#11677) * Linter manager (#11606) * Initial commit * Move event subscription to extension base class + add property changed filter to rules * comment updates * Update LinterExtensionBase.cs * Update InputNodesNotAllowedRule.cs * Add Deactivate method to linterExtensionBase + LinterManager tests * comment updates * Dispose LinterManager * comment updates * Serialize linter manager to dyn file * clear ruleEvaluationResults on workspace change * Remove test extension * Handle GraphRuleEvaluationResults nodeIds changes * Update SerializationConverters.cs * Fix failing serialization test * comment updates * revert changes to test file * Linter ViewExtension (#11634) * Initial commit * Move event subscription to extension base class + add property changed filter to rules * comment updates * Update LinterExtensionBase.cs * Update InputNodesNotAllowedRule.cs * Add Deactivate method to linterExtensionBase + LinterManager tests * comment updates * Initial commit * Add issues count to WorkspaceSaving * add scrollviewer * Dispose LinterManager * Make concrete issues internal * summaries on IRuleIssue * comment updates * Update severity code icon * Serialize linter manager to dyn file * clear ruleEvaluationResults on workspace change * Remove test extension * Show names instead of GUIDs * add help doc * clean up * Handle GraphRuleEvaluationResults nodeIds changes * Update LintingViewExtension.csproj * Update LintingViewExtension.csproj * Update SerializationConverters.cs * Fix failing serialization test * comment updates * revert changes to test file * fix available linters binding * add rules back in test ext * comment updates * remove output path * Update LinterManagerTests.cs * comment updates * hide LinterViewExtension and LinterExtensionBase * Update LinterViewModel.cs * if statements * remove Prism * clean up Co-authored-by: BogdanZavu <[email protected]>

SHKnudsen added 9 commits April 13, 2021 14:45

Initial commit

ceec5d6

Move event subscription to extension base class + add property change…

0057e7a

…d filter to rules

comment updates

668f4f9

Update LinterExtensionBase.cs

b69a7e9

Update InputNodesNotAllowedRule.cs

a1ad8c5

Add Deactivate method to linterExtensionBase + LinterManager tests

8ba184d

comment updates

c4c8286

Initial commit

d7c9537

Add issues count to WorkspaceSaving

1a58f63

SHKnudsen commented Apr 20, 2021

View reviewed changes

src/LintingViewExtension/LinterView.xaml Outdated Show resolved Hide resolved

add scrollviewer

d0bc4c7

mjkkirschner reviewed Apr 20, 2021

View reviewed changes

src/LintingViewExtension/Controls/GraphRuleIssueDto.cs Outdated Show resolved Hide resolved

mjkkirschner reviewed Apr 20, 2021

View reviewed changes

src/LintingViewExtension/LintingViewExtension.cs Outdated Show resolved Hide resolved

mjkkirschner reviewed Apr 20, 2021

View reviewed changes

SHKnudsen added 6 commits April 20, 2021 17:32

Dispose LinterManager

3a6f7af

Merge branch 'Linter-manager' into LinterViewExtension

60fb830

Make concrete issues internal

55ea85f

summaries on IRuleIssue

45edd6c

comment updates

aa58d7c

Update severity code icon

aee31ed

BogdanZavu reviewed Apr 27, 2021

View reviewed changes

src/LintingViewExtension/Controls/IRuleIssue.cs Show resolved Hide resolved

SHKnudsen added 4 commits April 27, 2021 19:50

Serialize linter manager to dyn file

fafba10

clear ruleEvaluationResults on workspace change

cdf0c59

Remove test extension

960c5b2

Show names instead of GUIDs

f0790d9

SHKnudsen added 4 commits April 28, 2021 13:19

add help doc

c89c476

clean up

558f05d

Handle GraphRuleEvaluationResults nodeIds changes

e7f2ff8

Update LintingViewExtension.csproj

f6bd740

revert changes to test file

45c8332

nate-peters reviewed Apr 30, 2021

View reviewed changes

src/LintingViewExtension/Docs/LinterViewHelpDoc.html Show resolved Hide resolved

SHKnudsen added 4 commits May 4, 2021 12:53

fix available linters binding

5304a08

Merge branch 'Linter-manager' into LinterViewExtension

93018d3

add rules back in test ext

17e9217

Merge branch 'AGD-2060-Linting-API' into LinterViewExtension

f0bec8c

BogdanZavu reviewed May 4, 2021

View reviewed changes

src/LintingViewExtension/LinterView.xaml Show resolved Hide resolved

SHKnudsen added 2 commits May 5, 2021 14:03

comment updates

69d7bf0

remove output path

4cc514a

SHKnudsen marked this pull request as ready for review May 5, 2021 15:30

Update LinterManagerTests.cs

a731f6c

comment updates

c7e3c19

SHKnudsen added 2 commits May 7, 2021 11:42

hide LinterViewExtension and LinterExtensionBase

902243a

Update LinterViewModel.cs

cbe848c

BogdanZavu approved these changes May 7, 2021

View reviewed changes

BogdanZavu merged commit cd60aef into DynamoDS:AGD-2060-Linting-API May 7, 2021

SHKnudsen mentioned this pull request May 7, 2021

[AGD-2060] Linting API #11673

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Linter ViewExtension #11634

Linter ViewExtension #11634

SHKnudsen commented Apr 20, 2021 •

edited

Loading

mjkkirschner Apr 20, 2021 •

edited

Loading

SHKnudsen May 5, 2021

mjkkirschner May 5, 2021

SHKnudsen May 5, 2021

SHKnudsen commented Apr 28, 2021 •

edited

Loading

BogdanZavu commented May 3, 2021

BogdanZavu May 4, 2021

BogdanZavu commented May 4, 2021 •

edited

Loading

BogdanZavu commented May 6, 2021

SHKnudsen commented May 6, 2021

BogdanZavu commented May 6, 2021 •

edited

Loading

mjkkirschner commented May 6, 2021 via email

SHKnudsen commented May 7, 2021

QilongTang commented May 7, 2021

SHKnudsen commented May 7, 2021

Linter ViewExtension #11634

Linter ViewExtension #11634

Conversation

SHKnudsen commented Apr 20, 2021 • edited Loading

Purpose

Linter ViewExtension

Declarations

Reviewers

FYIs

mjkkirschner Apr 20, 2021 • edited Loading

Choose a reason for hiding this comment

SHKnudsen May 5, 2021

Choose a reason for hiding this comment

mjkkirschner May 5, 2021

Choose a reason for hiding this comment

SHKnudsen May 5, 2021

Choose a reason for hiding this comment

SHKnudsen commented Apr 28, 2021 • edited Loading

Help link to Documentaiton Browser

BogdanZavu commented May 3, 2021

BogdanZavu May 4, 2021

Choose a reason for hiding this comment

BogdanZavu commented May 4, 2021 • edited Loading

BogdanZavu commented May 6, 2021

SHKnudsen commented May 6, 2021

BogdanZavu commented May 6, 2021 • edited Loading

mjkkirschner commented May 6, 2021 via email

SHKnudsen commented May 7, 2021

QilongTang commented May 7, 2021

SHKnudsen commented May 7, 2021

SHKnudsen commented Apr 20, 2021 •

edited

Loading

mjkkirschner Apr 20, 2021 •

edited

Loading

SHKnudsen commented Apr 28, 2021 •

edited

Loading

BogdanZavu commented May 4, 2021 •

edited

Loading

BogdanZavu commented May 6, 2021 •

edited

Loading