-
Notifications
You must be signed in to change notification settings - Fork 8
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
feat: apply fix command [IDE-976] #261
feat: apply fix command [IDE-976] #261
Conversation
plugin/src/main/java/io/snyk/eclipse/plugin/views/snyktoolview/BrowserHandler.java
Fixed
Show fixed
Hide fixed
plugin/src/main/java/io/snyk/eclipse/plugin/views/snyktoolview/BrowserHandler.java
Fixed
Show fixed
Hide fixed
plugin/src/main/java/io/snyk/eclipse/plugin/views/snyktoolview/BrowserHandler.java
Fixed
Show fixed
Hide fixed
public List<Fix> sendCodeFixDiffsCommand(String folderURI, String fileURI, String issueID) { | ||
// TODO: capture and return results | ||
executeCommand(LsConstants.COMMAND_CODE_FIX_DIFFS, List.of(folderURI, fileURI, issueID)); | ||
return null; |
Check warning
Code scanning / PMD
Return an empty collection rather than null.
66c8c9d
to
686e8c1
Compare
plugin/src/main/java/io/snyk/eclipse/plugin/views/snyktoolview/BrowserHandler.java
Fixed
Show fixed
Hide fixed
9f136b8
to
ee5f71f
Compare
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.
I think this is good, but it's a little difficult to review in isolation since it contains some of the changes from #260
Did you intend for this to be a diff on top of that one?
// Language server HTML assumes a base font size of 10px. The default Eclipse font size is 17px (13pt), so we | ||
// apply a scaling factor here. This ensures that HTML fonts scale correctly if the user changes the text size. | ||
// Language server HTML assumes a base font size of 10px. The default Eclipse | ||
// font size is 17px (13pt), so we |
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.
Odd line lengths here. Why are we limiting the width of comments and not the code?
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.
(This is a genuine question and not a criticism by the way; this was clearly done by some linter or script, and I want to make sure we're all being consistent)
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.
plugin/src/main/java/io/snyk/eclipse/plugin/html/CodeHtmlProvider.java
Outdated
Show resolved
Hide resolved
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.
This was part of your other PR - why is it changed in both?
@@ -73,6 +76,10 @@ class SnykExtendedLanguageClientTest extends LsBaseTest { | |||
|
|||
private ISnykToolView toolWindowMock; | |||
|
|||
private URI uri; | |||
private String paramName; | |||
// private SnykLogger loggerMock; |
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 either use or remove this
plugin/src/main/java/io/snyk/languageserver/protocolextension/SnykExtendedLanguageClient.java
Show resolved
Hide resolved
56bc98f
to
719420e
Compare
719420e
to
91c3093
Compare
01624d6
into
feat/IDE-957_code_action_opens_issue_panel
Description
Done:
Checklist
Screenshots / GIFs
Visuals that may help the reviewer. Please add screenshots for any UI change. GIFs are most welcome!