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

feat(vscode): make quick-lint a jerk 🖕 #1190

Closed
wants to merge 2 commits into from

Conversation

vegerot
Copy link
Contributor

@vegerot vegerot commented Jan 15, 2024

feat(vscode): make quick-lint a jerk 🖕

Summary: It ticks me off 😠 how polite 😇 quick-lint is when I code. If I mess
up 😬, I want it to tell me. Stop beating around the bush 🌳 and tell it to me
like it is. 💯

Test plan: 📝

  1. open TypeScript file with no errors in it ✅
  2. add an error ❌
  3. note how quick-lint tells you what the problem is like a sissy 😭
  4. enable settings > quick-lint-js > snarky 😈
  5. note how quick-lint is straight up with you. You suck. 🗑️

Closes #1188


Stack created with Sapling. Best reviewed with ReviewStack.

},
]
);
},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I should probably add enable_snarky_config_file and disable_snarky_config_file tests

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you can add quick-lint-js.config stuff into the other three tests.

Alternatively, you could data-drive the tests (see line 24 for an existing example):

for (let testCase of [
  {
    fileName: 'hello.js',
    content: 'undeclaredVariable',
    englishMessage: 'use of undeclared variable: undeclaredVariable',
    snarkyEnglishMessage: 'did you fail spelling class?',
  },
  {
    fileName: 'quick-lint-js.config',
    content: '{',
    englishMessage: '...',
    snarkyEnglishMessage: 'yeah, JSON sucks; try quick-lint-json',
  },
]) {
  tests = {
    ...tests,
    [`enabling snarky re-lints (${testCase.fileName})`]: async ({ addCleanup }) => {
      ...
    },
  };
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's an awesome idea 💡. This is why they pay you the big bucks

Copy link
Collaborator

@strager strager left a comment

Choose a reason for hiding this comment

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

Must fix: Remove profanity.

plugin/vscode/package.json Outdated Show resolved Hide resolved
plugin/vscode/quick-lint-js/vscode/qljs-document.h Outdated Show resolved Hide resolved
plugin/vscode/quick-lint-js/vscode/qljs-document.h Outdated Show resolved Hide resolved
plugin/vscode/quick-lint-js/vscode/qljs-document.h Outdated Show resolved Hide resolved
plugin/vscode/quick-lint-js/vscode/qljs-workspace.cpp Outdated Show resolved Hide resolved
plugin/vscode/test/vscode-tests.js Outdated Show resolved Hide resolved
message,
}));
assert.deepStrictEqual(diags, [
// use of undeclared variable 'undeclaredVariable'
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

plugin/vscode/test/vscode-tests.js Outdated Show resolved Hide resolved
plugin/vscode/test/vscode-tests.js Outdated Show resolved Hide resolved
},
]
);
},
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you can add quick-lint-js.config stuff into the other three tests.

Alternatively, you could data-drive the tests (see line 24 for an existing example):

for (let testCase of [
  {
    fileName: 'hello.js',
    content: 'undeclaredVariable',
    englishMessage: 'use of undeclared variable: undeclaredVariable',
    snarkyEnglishMessage: 'did you fail spelling class?',
  },
  {
    fileName: 'quick-lint-js.config',
    content: '{',
    englishMessage: '...',
    snarkyEnglishMessage: 'yeah, JSON sucks; try quick-lint-json',
  },
]) {
  tests = {
    ...tests,
    [`enabling snarky re-lints (${testCase.fileName})`]: async ({ addCleanup }) => {
      ...
    },
  };
}

Summary: This commit refactors the global translator class into a variable
injected into the VSCode workspace, and gives the workspace as an argument to
QLJS_Lintable_Document.

This refactor clears the way for adding snarky and language settings for VSCode
@vegerot vegerot changed the title feat(vscode): make quick-lint an asshole 🖕 feat(vscode): make quick-lint a jerk 🖕 Jan 21, 2024
@vegerot
Copy link
Contributor Author

vegerot commented Jan 21, 2024

Must fix: Remove profanity.

is "🖕🏾" profanity?

Summary: It ticks me off 😠 how polite 😇 quick-lint is when I code.  If I mess
up 😬, I want it to tell me.  Stop beating around the bush 🌳 and tell it to me
like it is. 💯

Test plan: 📝
1. open TypeScript file with no errors in it ✅
2. add an error ❌
3. note how quick-lint tells you what the problem is like a sissy 😭
4. enable settings > quick-lint-js > snarky 😈
5. note how quick-lint is straight up with you.  You suck. 🗑️

Closes quick-lint#1188
@strager
Copy link
Collaborator

strager commented Jan 25, 2024

Landed as Git commit 86bb25f.

@strager strager closed this Jan 25, 2024
this->translator_.use_messages_from_source_code();
}
this->qljs_documents_.for_each(
[this, is_snarky, env](::Napi::Value value) -> void {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Compiler was warning about is_snarky being an unused capture. I fixed it by using [&] instead.

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.

VSCode: support snarky option
2 participants