add support for fix all in file & project-info #234
Conversation
ananthakumaran
changed the title
|
It's not clear from the description, but if I were to test this... Would the simplest way to do so, would be with a TS-file with multiple unused imorts (as per microsoft/TypeScript#14549)? What other kinds of use-cases would be useful to verify? |
tide.el
Outdated
| (display-buffer (current-buffer) t))) | ||
|
|
||
| (defun tide-project-info () | ||
| "Show the version of tsserver." |
There was a problem hiding this comment.
I think this name is (currently) pretty misleading. It doesn't show much info about the project at all. What did you intend for this function to be used for? I think a naming based on that would work better.
For instance, if this is meant to aid diagnostics, we should call it tide-show-diagnostics or something similar.
There was a problem hiding this comment.
Yeah, it's mostly for debugging. project-info may be inaccurate. What about tide-verify-setup?. I am not sure about show-diagnostics
There was a problem hiding this comment.
I'm OK with that. That's still much better than tide-project-info.
tide.el
Outdated
| (tide-on-response-success response nil | ||
| (tide-apply-codefix (plist-get response :body))))) | ||
|
|
||
| (defun tide-apply-codefixes (fixes apply-codefix) | ||
| (cond ((= 0 (length fixes)) (message "No code-fixes available.")) |
There was a problem hiding this comment.
I think variable name apply-codefix is not very good. To me that name almost seems to imply this is a boolean, and IMO is a source of needless confusion or mental overhead.
codefix-function or something which hints at its type would be better.
tide.el
Outdated
| (defun tide-fix () | ||
| "Apply code fix for the error at point." | ||
| (interactive) | ||
| (defun tide-code-fix (apply-codefix) | ||
| (unless (tide-get-flycheck-errors-ids-at-point) |
There was a problem hiding this comment.
Same with the variable name apply-codefix here.
|
These are the couple of cases I have tested. // @ts-check
class C {
constructor() {
}
method() {
var x = this.x;
x += this.y();
x += this.x;
return x;
}
}export interface Shape {
getArea(): number;
}
export class Circle implements Shape {
}
class Rectangle implements Shape {
}
class Square implements Shape {
} |
|
Not a big difference in terms of code, but I still think that makes it much more readable and apparent what things are about :) |
|
have you had a chance to test this out? |
|
Sorry, but I simply haven't had time for that. Busy as heck at work. If it works for the use-cases you have intended, I say merge ahead. I have another few patches on another few projects which will probably get my attention first (which are also pending a proper review). |
Related to microsoft/TypeScript#20338. Need typescript@next for testing.