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: Add option in block-sequence-hyphen-indicator-newline #259

Conversation

sun-yryr
Copy link
Contributor

@sun-yryr sun-yryr commented Aug 6, 2023

This is my first pull request to this repository, so please let me know if I am doing something wrong.
Also, sorry if I used broken English.

why

JetBrains IDE can fix Yaml file as follows.

# Before
- !!str "a"
- 'b'
- &anchor "c"
- *anchor
- !!str [ 1, 2 ]
- { a: b }
- [ 1, 2 ]
- name: a
  value: b
- - a
  - b
# After
- !!str "a"
- 'b'
- &anchor "c"
- *anchor
- !!str [ 1, 2 ]
- { a: b }
- [ 1, 2 ]
-
  name: a
  value: b
-
  - a
  - b

Similar to rule block-sequence-hyphen-indicator-newline , but block-sequence-hyphen-indicator-newline actually adds a line break after all hyphen.

what

Add a new option blockMapping in block-sequence-hyphen-indicator-newline.
Defaults, this option is follows global style. If the option is set, it controls the line breaks in the block mapping accordingly.

@changeset-bot
Copy link

changeset-bot bot commented Aug 6, 2023

🦋 Changeset detected

Latest commit: 12784ac

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
eslint-plugin-yml Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@sun-yryr sun-yryr changed the title feat: block-sequence-hyphen-indicator-newlineにオプションを追加する feat: Add option in block-sequence-hyphen-indicator-newline Aug 6, 2023
@ota-meshi
Copy link
Owner

Thank you for this PR.

Could you please add to the rule documentation how the new options work?

"options": [
"never",
{
"nestedHyphen": "always",
Copy link
Owner

Choose a reason for hiding this comment

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

Please add test cases that does not use nestedHyphen.

@sun-yryr
Copy link
Contributor Author

Thank you for waiting long time.
Update document and add new test suites. Please review again.

Maybe resolve #243

@sun-yryr sun-yryr force-pushed the feature/option-block-sequence-hyphen-indicator-newline branch from 98e28ee to e1e11e7 Compare September 10, 2023 15:17
Copy link
Owner

@ota-meshi ota-meshi left a comment

Choose a reason for hiding this comment

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

When I checked the test cases, it seems that they do not include the block mapping test case.

Could you please add xx-input.yml under tests/fixtures/rules/block-sequence-hyphen-indicator-newline/invalid and tests/fixtures/rules/block-sequence-hyphen-indicator-newline/valid directory and write the config using comments at the beginning of files?

Please also refer to https://github.com/ota-meshi/eslint-plugin-yml/blob/master/tests/fixtures/rules/block-mapping/invalid/block-and-comment01-input.yml.

Copy link
Owner

@ota-meshi ota-meshi left a comment

Choose a reason for hiding this comment

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

Thank you!

@ota-meshi ota-meshi merged commit b73d852 into ota-meshi:master Sep 11, 2023
8 checks passed
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.

None yet

2 participants