Update ESLint configuration for json5 files #2317
Conversation
alanorth
added
i18n / l10n
|
I will restart the discussion on this after DSpace 7.6 gets released. In that time it would be good to think about the irregular space rule and the one blank line rule so we can start enforcing style in CI lint tests. |
alanorth
force-pushed
the
eslint-json5
branch
2 times, most recently
from
553419c to
1208385
Compare
tdonohue
requested review from
ybnd
and removed request for
alexandrevryghem
alanorth
force-pushed
the
eslint-json5
branch
from
dbcd5d0 to
7d8bb75
Compare
|
@alanorth some tests failing but a rebase will probably fix them i imagine. i'll give this a check |
alanorth
force-pushed
the
eslint-json5
branch
from
7d8bb75 to
9508498
Compare
|
Thanks @kshepherd. I've rebased on latest |
The eslint-plugin-jsonc has a recommended configuration for json5. See: https://ota-meshi.github.io/eslint-plugin-jsonc/user-guide/#usage
The eslint-plugin-jsonc documentation recommends turning ESLint's own no-irregular-whitespace plugin off for JSON files in favor of its own jsonc/no-irregular-whitespace plugin. The idea here is to allow "irregular" whitespace in quoted strings and regular expressions, as that may be deliberate, and to forbid such whitespace in comments and templates (though I'm unsure if we should skip them in templates?). See: https://ota-meshi.github.io/eslint-plugin-jsonc/rules/no-irregular-whitespace.html
Only allow one blank line in json5 files as well as a max of one line at the end of the file. See: https://eslint.org/docs/latest/rules/no-multiple-empty-lines
We want to avoid this:
{
// "bitstream.download.page.back": "Back" ,
"bitstream.download.page.back": "Atrás" ,
}
There is no need for the space before the comma. Note that we do
not need this plugin to check for spaces after the comma because
those will be highlighted by the other trailing whitespace checks.
See: https://eslint.org/docs/latest/rules/comma-spacing
alanorth
force-pushed
the
eslint-json5
branch
from
a4302aa to
55c9ca5
Compare
|
It looks like this JSONC validation behavior was added in #2110 by @alexandrevryghem . So, it'd be good to verify that switching to JSON5 validation should have no side-effects. @alexandrevryghem : your feedback on this PR would be welcome. @alanorth : I also notice we are still pulling in the Finally, in my opinion this must update the current JSON5 files...otherwise the |
This seems to have been added twice at some point.
Thanks @tdonohue. Well spotted! We need to keep one, as the
Sounds good to me. The JSON5 validation in the first commit will fix #2309, while the following three propose a more consistent and slightly stricter style with regards to whitespace:
Those are easily and automatically fixable so I think it's worth being consistent. |
References
Description
Update the ESLint configuration for json5 files to enforce a certain style that we can automatically verify and enforce for all future commits, pull requests, etc.
Note: after merging this, all i18n JSON5 files will need to be fixed using
yarn lint --fix, or it can be done as part of this.Instructions for Reviewers
List of changes in this PR:
plugin:jsonc/recommended-with-json5instead ofplugin:jsonc/recommended-with-jsoncjsonc/no-irregular-whitespaceplugin instead of ESLint'sno-irregular-whitespaceno-multiple-empty-linesplugin to enforce one blank line in between contentcomma-spacingplugin to disallow content likethis ,We need to decide:
npm run lint-fixto all i18n JSON5 files as part of this pull requestChecklist
yarn lintyarn check-circ-deps)package.json), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.