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

docs: Custom Processors cleanup and expansion #16838

Merged
merged 11 commits into from Mar 23, 2023

Conversation

bpmutter
Copy link
Contributor

Prerequisites checklist

What is the purpose of this pull request? (put an "X" next to an item)

[x] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofix to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:

What changes did you make? (Give an overview)

Clean up and expansion of the Custom Processors page.

Is there anything you'd like reviewers to focus on?

Resolves #16814

@eslint-github-bot eslint-github-bot bot added documentation Relates to ESLint's documentation triage An ESLint team member will look at this issue soon labels Jan 30, 2023
@netlify
Copy link

netlify bot commented Jan 30, 2023

Deploy Preview for docs-eslint ready!

Name Link
🔨 Latest commit c09e173
🔍 Latest deploy log https://app.netlify.com/sites/docs-eslint/deploys/64190fe936a2150008c628fa
😎 Deploy Preview https://deploy-preview-16838--docs-eslint.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@bpmutter bpmutter marked this pull request as ready for review February 1, 2023 02:40
@bpmutter bpmutter requested a review from a team as a code owner February 1, 2023 02:40
Copy link
Member

@nzakas nzakas left a comment

Choose a reason for hiding this comment

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

Nice cleanup! Hopefully my answers can help even more.

docs/src/extend/custom-processors.md Outdated Show resolved Hide resolved
docs/src/extend/custom-processors.md Outdated Show resolved Hide resolved
docs/src/extend/custom-processors.md Show resolved Hide resolved
@mdjermanovic mdjermanovic added accepted There is consensus among the team that this change meets the criteria for inclusion and removed triage An ESLint team member will look at this issue soon labels Feb 3, 2023
@bpmutter bpmutter requested a review from nzakas February 5, 2023 20:31
@bpmutter
Copy link
Contributor Author

bpmutter commented Feb 5, 2023

@nzakas implemented your feedback and added a few additional questions in comment threads

Copy link
Contributor

@snitin315 snitin315 left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -46,29 +47,28 @@ module.exports = {

**The `preprocess` method** takes the file contents and filename as arguments, and returns an array of code blocks to lint. The code blocks will be linted separately but still be registered to the filename.

A code block has two properties `text` and `filename`; the `text` property is the content of the block and the `filename` property is the name of the block. Name of the block can be anything, but should include the file extension, that would tell the linter how to process the current block. The linter will check [`--ext` CLI option](../use/command-line-interface#--ext) to see if the current block should be linted, and resolve `overrides` configs to check how to process the current block.
A code block has two properties `text` and `filename`. The `text` property is the content of the block and the `filename` property is the name of the block. The name of the block can be anything, but should include the file extension, which tells the linter how to process the current block. The linter checks the [`--ext` CLI option](../use/command-line-interface#--ext) to see if the current block should be linted and resolves `overrides` configs to check how to process the current block.
Copy link
Member

Choose a reason for hiding this comment

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

The last sentence here about checking --ext describes the pre-ESLint v6 behavior. That still happens, but it also checks extensions in overrides patterns now. That's described in https://eslint.org/docs/latest/use/configure/plugins#specify-a-processor.

If updating this is out of scope for the change you're intending here, feel free to disregard.

Copy link
Member

Choose a reason for hiding this comment

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

Nice catch! I couldn't remember if we had removed that functionality or not.

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 don't fully understand. would one of you mind providing a bit more info or a draft of what the copy should say here?

Copy link
Member

Choose a reason for hiding this comment

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

When you pass ESLint a folder on the command line, like eslint src/, ESLint walks the file system to find files to lint. By default, it finds .js, but you can override that in two different ways:

  1. By passing the --ext flag with different suffixes to find
  2. By specifying them in your config file through overrides patterns. Basically, each object in overrides has a files key that specifies glob patterns to match. These will be used to match inside of src/ in the previous example.

So --ext and overrides.files determines which blocks will be linted, and the other settings in overrides determine which configuration to apply to the blocks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thank you for clarifying!

Copy link
Member

Choose a reason for hiding this comment

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

To expand on what nzakas said and give an example, with the config below, to use the Markdown plugin prior to ESLint v6, one had to pass --ext .js,.md, or ESLint would only look at .js files. Now, ESLint will automatically see there's an overrides pattern for .md files and include them in the file traversal. One can still pass --ext to override that automatic behavior, but it in normal cases ESLint will do the right thing automatically.

// .eslintrc.js
module.exports = {
    plugins: ["markdown"],
    overrides: [
        {
            files: ["**/*.md"],
            processor: "markdown/markdown"
        }
    ]
};

Copy link
Contributor Author

Choose a reason for hiding this comment

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

do we think any further documentation is needed on this beyond what's in this PR?

my inclination is no, we do not. but maybe i'm not fullying grokking this.

Copy link
Member

Choose a reason for hiding this comment

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

I don't think we need anything further.

@bpmutter bpmutter requested review from nzakas and btmills and removed request for btmills and nzakas February 17, 2023 02:23
@@ -102,19 +139,41 @@ See [Specify a Processor](../use/configure/plugins#specify-a-processor) in the P

## File Extension-named Processor

If a processor name starts with `.`, ESLint handles the processor as a **file extension-named processor** especially and applies the processor to the kind of files automatically. People don't need to specify the file extension-named processors in their config files.
If a custom processor name starts with `.`, ESLint handles the processor as a **file extension-named processor**. ESLint applies the processor to files with that filename extension automatically. Users don't need to specify the file extension-named processors in their config files.

For example:

```js
module.exports = {
processors: {
// This processor will be applied to `*.md` files automatically.
Copy link
Member

Choose a reason for hiding this comment

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

@btmills just double-checking: is this still true? I seem to recall we might have removed automatically applied processors? Or maybe I'm just imagining it?

Copy link
Member

@btmills btmills Mar 11, 2023

Choose a reason for hiding this comment

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

It's at least still true under .eslintrc (implementation, tests). I don't know for sure that it's not part of flat config, but the tests all explicitly specify processor, and I can't find evidence of the back compat logic being carried over.

Should these docs discourage building file extension-named processors?

Copy link
Member

@nzakas nzakas left a comment

Choose a reason for hiding this comment

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

LGTM. Just leaving open to ensure @btmills' comments are incorporated.

Copy link
Member

@harish-sethuraman harish-sethuraman left a comment

Choose a reason for hiding this comment

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

LGTM leaving open for unresolved comments

Co-authored-by: Brandon Mills <btmills@users.noreply.github.com>
@bpmutter bpmutter requested a review from btmills March 11, 2023 20:30
@bpmutter bpmutter force-pushed the custom_processors_cleanup_expansion branch from ba9aaba to c09e173 Compare March 21, 2023 02:01
Copy link
Member

@nzakas nzakas left a comment

Choose a reason for hiding this comment

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

This looks ready to go. We can always adjust as needed.

@nzakas nzakas merged commit 721c717 into eslint:main Mar 23, 2023
33 checks passed
crapStone pushed a commit to Calciumdibromid/CaBr2 that referenced this pull request Mar 29, 2023
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [eslint](https://eslint.org) ([source](https://github.com/eslint/eslint)) | devDependencies | minor | [`8.36.0` -> `8.37.0`](https://renovatebot.com/diffs/npm/eslint/8.36.0/8.37.0) |

---

### Release Notes

<details>
<summary>eslint/eslint</summary>

### [`v8.37.0`](https://github.com/eslint/eslint/releases/tag/v8.37.0)

[Compare Source](eslint/eslint@v8.36.0...v8.37.0)

#### Features

-   [`b6ab8b2`](eslint/eslint@b6ab8b2) feat: `require-unicode-regexp` add suggestions ([#&#8203;17007](eslint/eslint#17007)) (Josh Goldberg)
-   [`10022b1`](eslint/eslint@10022b1) feat: Copy getScope() to SourceCode ([#&#8203;17004](eslint/eslint#17004)) (Nicholas C. Zakas)
-   [`1665c02`](eslint/eslint@1665c02) feat: Use plugin metadata for flat config serialization ([#&#8203;16992](eslint/eslint#16992)) (Nicholas C. Zakas)
-   [`b3634f6`](eslint/eslint@b3634f6) feat: docs license ([#&#8203;17010](eslint/eslint#17010)) (Samuel Roldan)
-   [`892e6e5`](eslint/eslint@892e6e5) feat: languageOptions.parser must be an object. ([#&#8203;16985](eslint/eslint#16985)) (Nicholas C. Zakas)

#### Bug Fixes

-   [`619f3fd`](eslint/eslint@619f3fd) fix: correctly handle `null` default config in `RuleTester` ([#&#8203;17023](eslint/eslint#17023)) (Brad Zacher)
-   [`1fbf118`](eslint/eslint@1fbf118) fix: `getFirstToken`/`getLastToken` on comment-only node ([#&#8203;16889](eslint/eslint#16889)) (Francesco Trotta)
-   [`129e252`](eslint/eslint@129e252) fix: Fix typo in `logical-assignment-operators` rule description ([#&#8203;17000](eslint/eslint#17000)) (Francesco Trotta)

#### Documentation

-   [`75339df`](eslint/eslint@75339df) docs: fix typos and missing info in id-match docs ([#&#8203;17029](eslint/eslint#17029)) (Ed Lucas)
-   [`ec2d830`](eslint/eslint@ec2d830) docs: Fix typos in the `semi` rule docs ([#&#8203;17012](eslint/eslint#17012)) (Andrii Lundiak)
-   [`e39f28d`](eslint/eslint@e39f28d) docs: add back to top button ([#&#8203;16979](eslint/eslint#16979)) (Tanuj Kanti)
-   [`721c717`](eslint/eslint@721c717) docs: Custom Processors cleanup and expansion ([#&#8203;16838](eslint/eslint#16838)) (Ben Perlmutter)
-   [`d049f97`](eslint/eslint@d049f97) docs: 'How ESLint is Maintained' page ([#&#8203;16961](eslint/eslint#16961)) (Ben Perlmutter)
-   [`5251a92`](eslint/eslint@5251a92) docs: Describe guard options for guard-for-in ([#&#8203;16986](eslint/eslint#16986)) (alope107)
-   [`6157d81`](eslint/eslint@6157d81) docs: Add example to guard-for-in docs. ([#&#8203;16983](eslint/eslint#16983)) (alope107)
-   [`fd47998`](eslint/eslint@fd47998) docs: update `Array.prototype.toSorted` specification link ([#&#8203;16982](eslint/eslint#16982)) (Milos Djermanovic)
-   [`3e1cf6b`](eslint/eslint@3e1cf6b) docs: Copy edits on Maintain ESLint docs ([#&#8203;16939](eslint/eslint#16939)) (Ben Perlmutter)

#### Chores

-   [`c67f299`](eslint/eslint@c67f299) chore: upgrade [@&#8203;eslint/js](https://github.com/eslint/js)[@&#8203;8](https://github.com/8).37.0 ([#&#8203;17033](eslint/eslint#17033)) (Milos Djermanovic)
-   [`ee9ddbd`](eslint/eslint@ee9ddbd) chore: package.json update for [@&#8203;eslint/js](https://github.com/eslint/js) release (ESLint Jenkins)
-   [`dddb475`](eslint/eslint@dddb475) chore: upgrade [@&#8203;eslint/eslintrc](https://github.com/eslint/eslintrc)[@&#8203;2](https://github.com/2).0.2 ([#&#8203;17032](eslint/eslint#17032)) (Milos Djermanovic)
-   [`522431e`](eslint/eslint@522431e) chore: upgrade espree@9.5.1 ([#&#8203;17031](eslint/eslint#17031)) (Milos Djermanovic)
-   [`f5f9a88`](eslint/eslint@f5f9a88) chore: upgrade eslint-visitor-keys@3.4.0 ([#&#8203;17030](eslint/eslint#17030)) (Milos Djermanovic)
-   [`4dd8d52`](eslint/eslint@4dd8d52) ci: bump actions/stale from 7 to 8 ([#&#8203;17026](eslint/eslint#17026)) (dependabot\[bot])
-   [`ad9dd6a`](eslint/eslint@ad9dd6a) chore: remove duplicate scss, ([#&#8203;17005](eslint/eslint#17005)) (Strek)
-   [`ada6a3e`](eslint/eslint@ada6a3e) ci: unpin Node 19 ([#&#8203;16993](eslint/eslint#16993)) (Milos Djermanovic)
-   [`c3da975`](eslint/eslint@c3da975) chore: Remove triage label from template ([#&#8203;16990](eslint/eslint#16990)) (Nicholas C. Zakas)
-   [`69bc0e2`](eslint/eslint@69bc0e2) ci: pin Node 19 to 19.7.0 ([#&#8203;16987](eslint/eslint#16987)) (Milos Djermanovic)

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNS4yNC41IiwidXBkYXRlZEluVmVyIjoiMzUuMjQuNSJ9-->

Co-authored-by: cabr2-bot <cabr2.help@gmail.com>
Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1834
Reviewed-by: Epsilon_02 <epsilon_02@noreply.codeberg.org>
Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
@eslint-github-bot eslint-github-bot bot locked and limited conversation to collaborators Sep 20, 2023
@eslint-github-bot eslint-github-bot bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Sep 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion documentation Relates to ESLint's documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Change Request: Custom Processors clean up and expansion
6 participants