Skip to content

Improve CS: declare variables individually - #3213

Merged
lubber-de merged 13 commits into
fomantic:developfrom
mvorisek:cs_var
Mar 15, 2025
Merged

Improve CS: declare variables individually#3213
lubber-de merged 13 commits into
fomantic:developfrom
mvorisek:cs_var

Conversation

@mvorisek

@mvorisek mvorisek commented Mar 13, 2025

Copy link
Copy Markdown
Contributor

This is default and prefered style beause:

  • the indentation is minimal
  • minimal diff on variable remove/add
  • less total LoC
  • var is used historically, my plan is to land this PR, and then detect not-redeclared variables and declare then using let/const

@lubber-de

Copy link
Copy Markdown
Member

According to https://eslint.org/docs/latest/rules/one-var the default for "one-var" is 'always' (you removed the one-var declaration in eslintrc) and "always" means to not declare each variable by a separate var/let/const ....
did i misunderstood the docs or did i miss something in your PR?

image
image

@lubber-de lubber-de added type/chore Anything which is a project chore type/lint eslint / stylelint related changes only and removed type/chore Anything which is a project chore labels Mar 13, 2025
@mvorisek

mvorisek commented Mar 13, 2025

Copy link
Copy Markdown
Contributor Author

You are right, I corrected the 1st commit title to mention "never". AirBnB/Unicorn default is newly used and the reasons why explained in the PR description.

@mvorisek
mvorisek force-pushed the cs_var branch 2 times, most recently from 857e2df to 0238ddc Compare March 13, 2025 12:56
@mvorisek

Copy link
Copy Markdown
Contributor Author

PR is done. I fixes the semicolon placements using regex manually, as semi-style rule removes wrongly new lines after them when autofixing.

@lubber-de

Copy link
Copy Markdown
Member

@mvorisek Please fix merge conflicts, as i just merged #3212 😉

@lubber-de

Copy link
Copy Markdown
Member

Again, please rebase / resolve conflicts, as i just merged your other PR #3215 🙂

Comment thread scripts/nightly-version.js
Comment thread tasks/config/admin/templates/less-package.js Outdated
regex: ((?:\n|^) *(let|const|var))\n *
regex: (?<=\n)( +(?! )).*(?<!{)\n(?!\1(?! )) *(/\*|//)
@mvorisek
mvorisek requested a review from lubber-de March 15, 2025 23:06
@mvorisek

mvorisek commented Mar 15, 2025

Copy link
Copy Markdown
Contributor Author

Let's please merge this PR before anything else to prevent conflicts.

I did a verification by replacing \s+ with \n after the 1st autofixed commit and after all commits. All non-whitespace changes are wanted, thus there is no functional change.

@lubber-de lubber-de left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@lubber-de
lubber-de merged commit c81d9eb into fomantic:develop Mar 15, 2025
@lubber-de lubber-de added this to the 2.10.0 milestone Mar 15, 2025
@mvorisek
mvorisek deleted the cs_var branch March 15, 2025 23:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type/lint eslint / stylelint related changes only

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants