add code fixes command #661
Conversation
TypeScript.sublime-commands
Outdated
| @@ -7,6 +7,7 @@ | |||
| { "caption" : "TypeScript: Format Selection", "command": "typescript_format_selection" }, | |||
| { "caption" : "TypeScript: Format Document", "command": "typescript_format_document" }, | |||
| { "caption" : "TypeScript: Find References", "command": "typescript_find_references" }, | |||
| { "caption" : "TypeScript: Code Fixes", "command": "typescript_get_code_fixes" }, | |||
There was a problem hiding this comment.
TypeScript: Request Code Fixes
typescript/libs/service_proxy.py
Outdated
| response_dict = self.__comm.sendCmdSync(json_str, req_dict["seq"]) | ||
| return response_dict | ||
|
|
||
|
|
typescript/libs/service_proxy.py
Outdated
| error['end']['offset'] >= column , errors_in_file['body'])) | ||
| return errors_at_cursor | ||
|
|
||
|
|
typescript/libs/service_proxy.py
Outdated
| column = cursor[1] + 1 | ||
| if errors_in_file['success']: | ||
| errors_at_cursor = list(filter(lambda error: | ||
| error['start']['line'] == line and |
typescript/libs/service_proxy.py
Outdated
| return response_dict | ||
|
|
||
|
|
||
| def get_errors_at_cursor(self, path, cursor): |
There was a problem hiding this comment.
I don't think get_errors_at_cursor should be in service_proxy.py. Also, to me it would make more sense to only request fixes at a position if the plugin has already reported a diagnostic on a given region (so you don't have to send a separate diagnostics request). In other words, I'm not sure requesting a code fix should request diagnostics.
Instead, maybe requesting a code fix at a position should check with the global ProjectErrorListener and see if it's been modified. If so, do nothing, but if not, filter through the errors as you've done here.
Curious to get @billti's thoughts here.
There was a problem hiding this comment.
After a bit of exploring
Current diagnostics only contain errorMessageString and region leaving out errorCodes and lineOffsets(which are both required arguments to getCodeFixes). I tried passing along errorCodes and offsets but it didn't come out well with existing code structure.
Definitely agree there is a wasted call to server, but for the amount of refactor and existing code base constraints it could be a bigger effort
I updated other things
typescript/libs/service_proxy.py
Outdated
| args = { | ||
| "file": path | ||
| } | ||
| req_dict = self.create_req_dict("semanticDiagnosticsSync", args) |
There was a problem hiding this comment.
This would also have to request syntactic diagnostics.
|
Looks sweet! I like how this just applies a quick fix if only one is available. However, I'm getting some strange behavior with carriage returns; notice the |
|
Which I looked at If the file doesn't have Unix line endings can you change it and see if that fixes it |
|
I made a code change to replace |
|
Is there anything else needed for this? |
|
Is there any way this can make it to the released package? It's working well for me when I manually replace the files in the sublime package directory |
|
Since @DanielRosenwasser review has been take into account, what is blocking this PR to be merged ? |
|
TBH, looks to me like all feedback is addressed and this PR should be merged 👍 |
fixes #630