-
Notifications
You must be signed in to change notification settings - Fork 175
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Hide the navigation nodes for unmodified nodes in diff #9015
base: main
Are you sure you want to change the base?
Hide the navigation nodes for unmodified nodes in diff #9015
Conversation
@@ -186,6 +186,9 @@ public void ConvertToTreeTokenModel() | |||
List<ReviewToken> currentLineTokens = new List<ReviewToken>(); | |||
foreach(var oldToken in Tokens) | |||
{ | |||
//Don't include documentation in converted code file due to incorrect documentation formatting used in previous model. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how did you find out this discrepancy? are there any automated tests we can run to keep this tool up to date with the design (as it changes)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also couldn't you convert the old documentation to new. Seem like that would be a straightforward mapping? Would be weird to just exclude documentation.
@@ -270,6 +273,13 @@ public void ConvertToTreeTokenModel() | |||
reviewLine.parentLine = parent; | |||
} | |||
|
|||
//Handle specific cases for C++ line with 'public:' and '{' to mark related line |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there another way we can guarantee this is only for c++? my worry is that another parser might have something similar and we don't know about and they run this tool and get a behavior that is not accurate
@@ -259,4 +263,18 @@ function shouldAppendIfRowIsHiddenAPI(row: CodePanelRowData) { | |||
} else { | |||
return true; | |||
} | |||
} | |||
|
|||
function FilterVisibleNavigationNodes(node: NavigationTreeNode) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should you make this private?
Hive the nodes if a corresponding code line is not visible in case of custom navigation tree. This PR also has the changes to exclude documentation when converting tokens from old model to streamline the token structure. Old token model has incorrect documentation line with partial code line.