Add commands for navigating the steps on a path#175
Conversation
jcreedcmu
left a comment
There was a problem hiding this comment.
Mostly lgtm, just a couple of optional nits.
| export function getResult(sarif: sarif.Log, key: Result): sarif.Result | undefined { | ||
| if (sarif.runs.length === 0) return undefined; | ||
| if (sarif.runs[0].results === undefined) return undefined; | ||
| let results = sarif.runs[0].results; |
There was a problem hiding this comment.
This can be const, yeah?
| super(props); | ||
| this.state = { expanded: {} }; | ||
| this.state = { expanded: {}, selectedPathNode: undefined }; | ||
| this.handleNavigationEvent = this.handleNavigationEvent.bind(this); |
There was a problem hiding this comment.
I almost prefer to have the .bind(this)s at the callsite, or else give the property the .bind()ed method a different name from the original method name. Do you have a strong opinion here? I haven't seen this pattern before but am willing to defer if you feel like it's idiomatic.
There was a problem hiding this comment.
I'd say it's quite common. It's recommended on reactjs.org, with the other alternative being to make the method into a field instead.
We generally recommend binding in the constructor or using the class fields syntax, to avoid this sort of performance problem.
This adds the command
CodeQL: Show Next Step on PathandCodeQL: Show Previous Step on Pathfor navigating the steps on the currently shown path.The idea is that you can add keybindings for these commands to navigate the path without the mouse. Doing this with the mouse required quite a bit of zig-zag eye tracking between the code view and result view.
You still have to click one of the steps on a path to "choose" that path. There are currently no commands for switching between results or paths. It would be nice to have a fully keyboard-controllable result view, but I think I'll leave that for a future PR.
It's a bit of a hassle to make the result view remember which path node was previously clicked. For this we need a way to reference a specific part of the result set, that is, a result index, path index, and a path step index. I introduced the types
Keys.Result,Keys.Path, andKeys.PathNodeto represent such indices.Commit-by-commit review recommended.