Give rules access to settings #279

jablko · 2022-02-17T21:20:40Z

Initial checklist

I read the support docs
I read the contributing guide
I agree to follow the code of conduct
I searched issues and couldn’t find anything (or linked relevant results below)
If applicable, I’ve added docs and tests

Description of changes

I'm interested in working on this: Giving rules access to settings.

Also, some rules might need access to the fileSet too? but from what I could gather, while settings is accessible via this (processor: this.data().settings), fileSet isn't? Consequently I've appended it to the rule arguments, symmetric with how the other (options) attacher parameter is appended to the rule arguments, roughly.

Fixes #263 (comment)

ChristianMurphy

Thanks @jablko!
It's good to see #279 moving forward, thanks for taking time to look into it.

A general note, there seem to be some changes which, at first blush appear to be unrelated mixed in here.
Some documentation changes, and (semver) major changes to how settings directly passed are handled.
Are those intentional? Could you expand a bit on how they tie into this PR and what the goal/intention with the additional changes is?

so, some rules might need access to the fileSet too

Could you expand on this?
In general Unified/remark/remark-lint acts on a single file at a time.
There has been some thoughts on a higher level tool which would handle multiple files unifiedjs/ideas#11 (comment), but that would likely live at a higher level of abstraction?

packages/remark-lint-blockquote-indentation/readme.md

ChristianMurphy · 2022-02-20T18:34:58Z

packages/remark-lint-emphasis-marker/index.js

@@ -40,32 +40,32 @@
 * @copyright 2015 Titus Wormer
 * @license MIT
 * @example
- *   {"setting": "*", "name": "ok.md"}
+ *   {"settings": {"emphasis": "*"}, "name": "ok.md"}


this appears to suggest a breaking change?
That all configuration would need to be updated to this format?
Is that accurate?

ChristianMurphy · 2022-02-20T18:37:18Z

packages/remark-lint-first-heading-level/readme.md

@@ -135,14 +135,10 @@ a value of `2` can be defined here.

 ##### `ok.md`

-When configured with `2`.


again, I'm a bit confused.
These changes appear to be unrelated to the settings, and it's unclear what these changes fix/improve.

ChristianMurphy · 2022-02-20T18:38:54Z

script/util/rule.js

@@ -77,7 +77,7 @@ export function rule(filePath) {

  while (++index < examples.length) {
    const lines = examples[index].split('\n')
-    /** @type {{name: string, label?: 'input'|'output', setting?: unknown, positionless?: boolean, gfm?: boolean}} */
+    /** @type {{name: string, label?: 'input'|'output', settings?: unknown, setting?: unknown, positionless?: boolean, gfm?: boolean}} */


this seems like a typo, defining settings twice

settings is the processor.use({settings}) argument vs. setting is the processor.use(rule, setting) argument.

jablko · 2022-02-20T20:59:28Z

@ChristianMurphy Thank you for the encouragement!

I started out aiming to give rules access to settings --- but to actually fix #263 (comment), I additionally made the remark-lint-emphasis-marker and remark-lint-strong-marker options default to settings.emphasis and settings.strong, respectively:

remark-lint/packages/remark-lint-emphasis-marker/index.js

Lines 108 to 111 in 8b57de8

    
           if (!option) { 
        
             const settings = this.data('settings') 
        
             option = (settings && settings.emphasis) || 'consistent' 
        
           }

I've now added a commit to update those docs. 👍

jablko · 2022-02-20T21:08:05Z

unified-engine passes options and fileSet to the attacher function. Plugins use it. With this change, rules can too?

.github/workflows/main.yml

jablko · 2022-02-21T00:25:01Z

I've added a commit to make the remaining rules' options default to their respective settings.

wooorm · 2022-03-24T10:11:58Z

@jablko How’s it going here? Can you let me know if you need help or if there’s anything in particular to review?

jablko

Thanks for the helpful and detailed feedback!

jablko · 2022-03-30T16:33:33Z

readme.md

+      // recommended preset, but if we’d prefer something else, it can be
+      // reconfigured:
+      settings: {listItemIndent: 'one'}
+    })


I've edited the prose:

settings.listItemIndent is set to tab in the recommended preset, but you can change it if you’d prefer something else:

jablko · 2022-03-30T16:35:34Z

readme.md

+
+*   Pass a severity, options, or both to the rules themselves.
+    This affects each rule individually and takes highest precedence.
+*   Some rules correspond to settings that are shared among all remark plugins --- particularly [`remark-stringify`](https://github.com/remarkjs/remark/tree/main/packages/remark-stringify#options).


I've added shared settings docs to the Configure section.

jablko · 2022-03-30T16:36:03Z

packages/remark-lint-code-block-style/readme.md

 to always use fenced code.

 ## Examples

 ##### `ok.md`

-When configured with `'indented'`.
+When [`settings.fences`](https://github.com/remarkjs/remark-lint#configure) is `false` and the rule is not configured.


In each readme I've linked to the monorepo Configure section.

jablko · 2022-03-30T16:36:33Z

readme.md

+| Setting | Rule |
+| - | - |
+| [`settings.bullet`](https://github.com/remarkjs/remark/tree/main/packages/remark-stringify#optionsbullet) | [`remark-lint-unordered-list-marker-style`](https://github.com/remarkjs/remark-lint/tree/main/packages/remark-lint-unordered-list-marker-style) |
+| [`settings.bulletOrdered`](https://github.com/remarkjs/remark/tree/main/packages/remark-stringify#optionsbulletordered) | [`remark-lint-ordered-list-marker-style`](https://github.com/remarkjs/remark-lint/tree/main/packages/remark-lint-ordered-list-marker-style) |
+| [`settings.closeAtx`](https://github.com/remarkjs/remark/tree/main/packages/remark-stringify#optionscloseatx) | [`remark-lint-heading-style`](https://github.com/remarkjs/remark-lint/tree/main/packages/remark-lint-heading-style) |
+| [`settings.emphasis`](https://github.com/remarkjs/remark/tree/main/packages/remark-stringify#optionsemphasis) | [`remark-lint-emphasis-marker`](https://github.com/remarkjs/remark-lint/tree/main/packages/remark-lint-emphasis-marker) |
+| [`settings.fence`](https://github.com/remarkjs/remark/tree/main/packages/remark-stringify#optionsfence) | [`remark-lint-fenced-code-marker`](https://github.com/remarkjs/remark-lint/tree/main/packages/remark-lint-fenced-code-marker) |
+| [`settings.fences`](https://github.com/remarkjs/remark/tree/main/packages/remark-stringify#optionsfences) | [`remark-lint-code-block-style`](https://github.com/remarkjs/remark-lint/tree/main/packages/remark-lint-code-block-style) |
+| [`settings.incrementListMarker`](https://github.com/remarkjs/remark/tree/main/packages/remark-stringify#optionsincrementlistmarker) | [`remark-lint-ordered-list-marker-value`](https://github.com/remarkjs/remark-lint/tree/main/packages/remark-lint-ordered-list-marker-value) |
+| [`settings.listItemIndent`](https://github.com/remarkjs/remark/tree/main/packages/remark-stringify#optionslistitemindent) | [`remark-lint-list-item-indent`](https://github.com/remarkjs/remark-lint/tree/main/packages/remark-lint-list-item-indent) |
+| [`settings.quote`](https://github.com/remarkjs/remark/tree/main/packages/remark-stringify#optionsquote) | [`remark-lint-link-title-style`](https://github.com/remarkjs/remark-lint/tree/main/packages/remark-lint-link-title-style) |
+| [`settings.rule`](https://github.com/remarkjs/remark/tree/main/packages/remark-stringify#optionsrule) | [`remark-lint-rule-style`](https://github.com/remarkjs/remark-lint/tree/main/packages/remark-lint-rule-style) |
+| [`settings.ruleRepetition`](https://github.com/remarkjs/remark/tree/main/packages/remark-stringify#optionsrulerepetition) | [`remark-lint-rule-style`](https://github.com/remarkjs/remark-lint/tree/main/packages/remark-lint-rule-style) |
+| [`settings.ruleSpaces`](https://github.com/remarkjs/remark/tree/main/packages/remark-stringify#optionsrulespaces) | [`remark-lint-rule-style`](https://github.com/remarkjs/remark-lint/tree/main/packages/remark-lint-rule-style) |
+| [`settings.setext`](https://github.com/remarkjs/remark/tree/main/packages/remark-stringify#optionssetext) | [`remark-lint-heading-style`](https://github.com/remarkjs/remark-lint/tree/main/packages/remark-lint-heading-style) |
+| [`settings.strong`](https://github.com/remarkjs/remark/tree/main/packages/remark-stringify#optionsstrong) | [`remark-lint-strong-marker`](https://github.com/remarkjs/remark-lint/tree/main/packages/remark-lint-strong-marker) |


I've added a table to the Configure section, listing the settings that also affect lint rules.

jablko · 2022-03-30T16:36:47Z

readme.md

+> 🧑‍🏫 **Info**: remark lint rules *check* markdown.
+> [`remark-stringify`][remark-stringify] *formats* markdown.
+> They behave consistently when you change shared settings that they both understand, but not if you pass options to rules individually.
+> Passing options to rules individually doesn't affect [`remark-stringify`][remark-stringify] or vice versa.


I've added the trade-offs between rules' options and shared settings to the Configure section, and called out the synchronization problem.

jablko · 2022-03-30T16:37:07Z

packages/remark-preset-lint-markdown-style-guide/readme.md

@@ -141,24 +138,24 @@ This preset configures [`remark-lint`][mono] with the following rules:
 | [`remark-lint-maximum-line-length`](https://github.com/remarkjs/remark-lint/tree/main/packages/remark-lint-maximum-line-length) | `80` |
 | [`remark-lint-no-shell-dollars`](https://github.com/remarkjs/remark-lint/tree/main/packages/remark-lint-no-shell-dollars) | |
 | [`remark-lint-hard-break-spaces`](https://github.com/remarkjs/remark-lint/tree/main/packages/remark-lint-hard-break-spaces) | |
-| [`remark-lint-heading-style`](https://github.com/remarkjs/remark-lint/tree/main/packages/remark-lint-heading-style) | `'atx'` |
+| [`remark-lint-heading-style`](https://github.com/remarkjs/remark-lint/tree/main/packages/remark-lint-heading-style) | [`settings.setext`](https://github.com/remarkjs/remark-lint#configure) is `false` |


I've distinguished between rules' options and shared settings in the presets' readmes.

jablko · 2022-03-30T16:37:25Z

script/build-rules.js

+          const {settings, rawOptions} = JSON.parse(configuration)
+          const fixtures = tests[configuration]
+
+          // Don't show the same test for both shared settings and rules' options.


I've hidden repetitious examples from the readmes:

All of the existing tests are retained.

Each readme has the same number of examples as it does now.

Co-authored-by: Titus <[email protected]>

This reverts commit 7d24ed2.

JounQin · 2022-07-26T15:32:31Z

Are we moving on this?

cc @wooorm

wooorm · 2022-07-27T16:47:26Z

It’s been a while!

@jablko Was this done? Ready for another review? Anything in particular I can help with?

romgere · 2023-03-17T09:10:38Z

Any update on this 🤔

github-actions bot added the 👋 phase/new Post is being triaged automatically label Feb 17, 2022

This comment has been minimized.

Sign in to view

jablko force-pushed the patch-1 branch 4 times, most recently from ca0bfba to 9f2c248 Compare February 17, 2022 22:36

This comment was marked as resolved.

Sign in to view

jablko force-pushed the patch-1 branch 2 times, most recently from d816196 to f5ddf64 Compare February 17, 2022 23:03

jablko mentioned this pull request Feb 19, 2022

Add support for this in wrap wooorm/trough#11

Merged

jablko force-pushed the patch-1 branch from f5ddf64 to 47b4250 Compare February 19, 2022 20:41

github-actions bot added 🤞 phase/open Post is being triaged manually and removed 👋 phase/new Post is being triaged automatically labels Feb 19, 2022

jablko force-pushed the patch-1 branch 8 times, most recently from deed8ce to ef344b4 Compare February 20, 2022 18:18

ChristianMurphy reviewed Feb 20, 2022

View reviewed changes

ChristianMurphy added the 🗄 area/interface This affects the public interface label Feb 20, 2022

jablko force-pushed the patch-1 branch from ef344b4 to d7dfe59 Compare February 20, 2022 20:57

jablko commented Feb 20, 2022

View reviewed changes

.github/workflows/main.yml Outdated Show resolved Hide resolved

jablko force-pushed the patch-1 branch 2 times, most recently from c6a986f to 20685cf Compare February 22, 2022 16:46

jablko force-pushed the patch-1 branch from 0bebc1a to 4194ac5 Compare March 23, 2022 22:55

jablko force-pushed the patch-1 branch 5 times, most recently from ad97cc3 to c59a20a Compare March 28, 2022 15:25

jablko commented Mar 30, 2022

View reviewed changes

This was referenced Mar 30, 2022

Update build scripts to use improved settings #286

Merged

Add settings for enforced styles to presets #282

Open

jablko force-pushed the patch-1 branch from c59a20a to cad7332 Compare March 30, 2022 19:49

jablko and others added 15 commits March 31, 2022 14:42

Give rules access to settings

f4dd4ee

Add settings to tests

2a3cb2d

Update docs

b870ec2

Harmonize list item indent literals

04cdfbd

Make rule options default to respective settings

aad72a4

Add settings for enforced styles to presets

fe4deff

Add settings to docs

ec6809e

Replace preset rule options with settings

bbcffa7

Restore existing tests

98097b5

Undefined settings style

06627f2

Restore preset settings docs

a9caeeb

Revise fixture descriptions

8acf647

Apply suggestions from code review

08ee180

Co-authored-by: Titus <[email protected]>

Revise docs

6344198

Revert "Harmonize list item indent literals"

1f1c446

This reverts commit 7d24ed2.

jablko force-pushed the patch-1 branch from cad7332 to 1f1c446 Compare March 31, 2022 21:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Give rules access to settings #279

Give rules access to settings #279

jablko commented Feb 17, 2022 •

edited

Loading

This comment has been minimized.

This comment was marked as resolved.

ChristianMurphy left a comment

ChristianMurphy Feb 20, 2022

ChristianMurphy Feb 20, 2022

ChristianMurphy Feb 20, 2022

jablko Feb 20, 2022 •

edited

Loading

jablko commented Feb 20, 2022 •

edited

Loading

jablko commented Feb 20, 2022

jablko commented Feb 21, 2022

wooorm commented Mar 24, 2022

jablko left a comment

jablko Mar 30, 2022

jablko Mar 30, 2022

jablko Mar 30, 2022

jablko Mar 30, 2022

jablko Mar 30, 2022

jablko Mar 30, 2022

jablko Mar 30, 2022

JounQin commented Jul 26, 2022 •

edited

Loading

wooorm commented Jul 27, 2022

romgere commented Mar 17, 2023

		@@ -135,14 +135,10 @@ a value of `2` can be defined here.

		##### `ok.md`

		When configured with `2`.

Give rules access to settings #279

Are you sure you want to change the base?

Give rules access to settings #279

Conversation

jablko commented Feb 17, 2022 • edited Loading

Initial checklist

Description of changes

This comment has been minimized.

This comment was marked as resolved.

ChristianMurphy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jablko Feb 20, 2022 • edited Loading

Choose a reason for hiding this comment

jablko commented Feb 20, 2022 • edited Loading

jablko commented Feb 20, 2022

jablko commented Feb 21, 2022

wooorm commented Mar 24, 2022

jablko left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JounQin commented Jul 26, 2022 • edited Loading

wooorm commented Jul 27, 2022

romgere commented Mar 17, 2023

jablko commented Feb 17, 2022 •

edited

Loading

jablko Feb 20, 2022 •

edited

Loading

jablko commented Feb 20, 2022 •

edited

Loading

JounQin commented Jul 26, 2022 •

edited

Loading