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

Add lint on type and other improvements #145

Closed

Conversation

nobodywasishere
Copy link
Contributor

@nobodywasishere nobodywasishere commented Nov 17, 2024

Closes #89.

  • Adds a config option lint-trigger that defaults to type, but will be set to save for versions of ameba < 1.6.3
  • Adds a config option lint-scope that tells the extension how to treat the linter
    • file is the current behavior of only running the linter on open files, and removing lints when files are closed
    • workspace runs lints for the entire workspace, and doesn't remove them when the file is closed; I personally prefer this mode as it allows me to see lints for files I haven't touched
  • Moves to using spawn instead of exec as it's required to be able to write to stdin
  • Adds an output channel to know what the extension is doing, to help with debugging and to make sure it behaves
  • Disables certain rules by default when typing as they're going to show up (it gets annoying)
  • I was running into a lot of issues with tasks just not clearing from the queue, so I replaced TaskToken/CancelCallback with the built-in vscode CancellationToken, which worked a lot better
  • This does add a dependency on semver, which may or may not be desired; if it's not, its functionality can be rewritten locally
  • Modifying the .ameba.yml will clear all diagnostics from all documents and re-run ameba on them to refresh their diagnostics with the new rules

@nobodywasishere
Copy link
Contributor Author

Apologies for how big this PR is. If you'd prefer it split up, I can do that.

@veelenga
Copy link
Member

veelenga commented Nov 17, 2024

It is not that big. Could you please rebase it, I will continue review/test tomorrow? Thanks for the PR 💪🏻

Switched from exec to spawn to use stdin, and we don't have to worry about paths with spaces
Replaced TaskToken/CancelCallback with vscode CancellationToken due to running into queue issues
Added output channel to see what the extension is doing to help w/ debugging
onType is enabled by default for ameba >=1.6.3
saving a `.ameba.yml` in a workspace will clear all diagnostics from all documents and re-run ameba on them

The latter allows for a very fluid experience of editing the config and seeing the errors in real time
package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
src/ameba.ts Outdated Show resolved Hide resolved
src/configuration.ts Outdated Show resolved Hide resolved
src/extension.ts Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
nobodywasishere and others added 3 commits November 17, 2024 18:19
Co-authored-by: Sijawusz Pur Rahnama <[email protected]>
Co-authored-by: Sijawusz Pur Rahnama <[email protected]>
Co-authored-by: Sijawusz Pur Rahnama <[email protected]>
src/configuration.ts Outdated Show resolved Hide resolved
Co-authored-by: Sijawusz Pur Rahnama <[email protected]>
src/ameba.ts Outdated Show resolved Hide resolved
Co-authored-by: Sijawusz Pur Rahnama <[email protected]>
src/ameba.ts Outdated Show resolved Hide resolved
src/ameba.ts Outdated Show resolved Hide resolved
package.json Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
src/ameba.ts Outdated Show resolved Hide resolved
src/configuration.ts Outdated Show resolved Hide resolved
src/extension.ts Outdated Show resolved Hide resolved
src/extension.ts Outdated Show resolved Hide resolved
Co-authored-by: Sijawusz Pur Rahnama <[email protected]>
src/configuration.ts Outdated Show resolved Hide resolved
nobodywasishere and others added 2 commits November 17, 2024 19:17
Co-authored-by: Sijawusz Pur Rahnama <[email protected]>
Co-authored-by: Sijawusz Pur Rahnama <[email protected]>
src/taskQueue.ts Outdated Show resolved Hide resolved
src/ameba.ts Show resolved Hide resolved
src/extension.ts Outdated Show resolved Hide resolved
package.json Show resolved Hide resolved
…pe config option

lint-scope of file is how the ext currently behaves, clearing diagnostics if a file is closed and only linting open files

lint-scope of workspace runs ameba over the entire workspace, not clearing them when a file is closed
Copy link
Member

@Sija Sija left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's GTG from me, once the several remaining points are addressed.

@nobodywasishere
Copy link
Contributor Author

It's GTG from me, once the several remaining points are addressed.

I've actually made a few changes locally that will need review as well, primarily around onSave/onType config. I should be able to push in a few hours.

src/configuration.ts Outdated Show resolved Hide resolved
@nobodywasishere
Copy link
Contributor Author

Updated with most recent changes, also updated the description

@Sija
Copy link
Member

Sija commented Nov 22, 2024

@nobodywasishere This PR got pretty big with the recent changes. It would be much easier to review it in parts, would it be possible for you to split it to smaller PRs in order to help with the process?

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.

Add option to lint while typing instead of waiting for file save
3 participants