Skip to content
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

Fix all and parser errors handler #92

Merged
merged 3 commits into from
Apr 22, 2023
Merged

Fix all and parser errors handler #92

merged 3 commits into from
Apr 22, 2023

Conversation

RblSb
Copy link
Member

@RblSb RblSb commented Apr 3, 2023

Can be tested with settings.json:

"editor.codeActionsOnSave": {
	"source.sortImports": true,
	"source.fixAll": true
},

Fixes missing semicolons, commas, overrides and changes final var to final for now.
Depends on vshaxe/vshaxe#576

I think we need to name all code actions like this: https://dartcode.org/docs/refactorings-and-code-fixes/
And then add enum for fixAll command in settings to make customizable list of autofixes (instead of current quickfix.auto kind)?

Also i need some explanation what two first early returns in ParserErrorActions are doing, this is copypasted from other file.

@AlexHaxe
Copy link
Member

AlexHaxe commented Apr 4, 2023

after playing around with it, I think the parser error stuff is fine (although I haven't tried to recreate all different error conditions).

the fix all part is somehow misbehaving for me. it doesn't show up in the quickfix popup unless I do '<right click> -> Source Action... '. when I put "source.fixAll": true in settings, it will correct one error on each save, and if I have two missing semicolons, I end up having three after three saves (it goes ;; on one of the two places somehow).
also Fix All source action will show up regardless of things to fix.
another question is how do other editors handle custom commands, will they just ignore them or throw an error?

since we'll have to release 2.28.0 soon, I would suggest splitting your PR in two. one for parser errors and one for "fix all". so we can include parser error handling in 2.28.0 and can deal with fix all later.

@RblSb RblSb mentioned this pull request Apr 4, 2023
@RblSb
Copy link
Member Author

RblSb commented Apr 4, 2023

Are you sure Fix All should popup as suggestion for any error? Maybe this is hidden on vscode level? I cannot get TS to fix missing commas (tested with new typescript 5 template, maybe i'm missing something), and Dart does not have working autofixes for ;. Need some language as good example.
About one error on each save - this is how compiler reports missing commas, and i think this is good enough when you write code anyway.
I need some info about ;;, is this happens when you resave too fast, or maybe with some complicated blocks?
For me this can only be reproduced if you call action multiple times without saving file in between, but this problem happens with most actions, ideally we need to block them on client when editor isDirty and provide option to autosave after action is done. This also depends on diagnostic updates only after save.
I can basically check for ; token anyway and skip adding second one, i guess.
Custom commands is things from other extensions? Do not use anything, but maybe naming actions with unique sub-kinds will make it safe.

@AlexHaxe
Copy link
Member

AlexHaxe commented Apr 4, 2023

Are you sure Fix All should popup as suggestion for any error? Maybe this is hidden on vscode level? I cannot get TS to fix missing commas (tested with new typescript 5 template, maybe i'm missing something), and Dart does not have working autofixes for ;. Need some language as good example.

I'm not sure if it should pop up, I'm just wondering why it doesn't, since it gets added regardless of number of fixes.
it would make sense to show up, if there is more than one issue that could be fixed by it.

About one error on each save - this is how compiler reports missing commas, and i think this is good enough when you write code anyway.

I assume that applies to most if not all parser errors, so a fix all is a bit misleading for them, until we have more things that get fixed in the future. I'm not sure what issues we can include in these fix all / auto-fixes, since they would have to be ones that don't require user input.

I need some info about ;;, is this happens when you resave too fast, or maybe with some complicated blocks? For me this can only be reproduced if you call action multiple times without saving file in between, but this problem happens with most actions, ideally we need to block them on client when editor isDirty and provide option to autosave after action is done. This also depends on diagnostic updates only after save. I can basically check for ; token anyway and skip adding second one, i guess.

I didn't resave very fast, however somehow the last parser error remains after saving sometimes, so the third save would still see that parser error and add a second ;. it doesn't happen all the time, but occasionally.

Custom commands is things from other extensions? Do not use anything, but maybe naming actions with unique sub-kinds will make it safe.

I'm unsure on that, maybe someone using an alternative editor can chime in.

@RblSb RblSb force-pushed the fix_all branch 2 times, most recently from 88fd7d2 to 1ac2131 Compare April 5, 2023 00:14
@AlexHaxe AlexHaxe merged commit eb9176c into vshaxe:master Apr 22, 2023
@RblSb RblSb deleted the fix_all branch April 22, 2023 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants