Skip to content

vscode: update editor settings and commit constraints#28306

Merged
hnyman merged 1 commit intoopenwrt:masterfrom
BKPepe:more-vscode-changes
Mar 13, 2026
Merged

vscode: update editor settings and commit constraints#28306
hnyman merged 1 commit intoopenwrt:masterfrom
BKPepe:more-vscode-changes

Conversation

@BKPepe
Copy link
Member

@BKPepe BKPepe commented Jan 9, 2026

Enable trailing whitespace trimming, insert final newline, and force LF. Configure git input validation to warn if subject exceeds 60 characters or if body lines exceed 75 characters.

More changes to file, which was added in #25034

cc: @bam80

Enable trailing whitespace trimming, insert final newline, and force LF.
Configure git input validation to warn if subject exceeds 60 characters
or if body lines exceed 75 characters.

Signed-off-by: Josef Schlehofer <[email protected]>
Copilot AI review requested due to automatic review settings January 9, 2026 14:07
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR enhances the VS Code workspace configuration by adding editor file formatting settings and git commit message validation rules to ensure consistent code quality and proper commit message formatting.

  • Enables automated file cleanup (trailing whitespace trimming, final newline insertion)
  • Enforces LF line endings for cross-platform consistency
  • Configures git commit message length validation (60 char subject, 75 char body lines)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

} No newline at end of file
"git.alwaysSignOff": true,
"files.trimTrailingWhitespace": true,
"files.insertFinalNewline": true,
Copy link
Contributor

Choose a reason for hiding this comment

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

Never understood why it's needed.

Copy link
Member

Choose a reason for hiding this comment

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

echo -n a > a
echo a > b
echo b >> b
diff -u a b
--- a   2026-01-09 23:39:47.701093532 +0200
+++ b   2026-01-09 23:39:47.701093532 +0200
@@ -1 +1,2 @@
-a
\ No newline at end of file
+a
+b
echo a > a
echo a > b
echo b >> b
diff -u a b
--- a   2026-01-09 23:40:21.501894017 +0200
+++ b   2026-01-09 23:40:21.501894017 +0200
@@ -1 +1,2 @@
 a
+b

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, I never felt in need of that for myself, but if you think it's worth it - go ahead.

Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't it the default behavior anyway?

Copy link
Member Author

Choose a reason for hiding this comment

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

In practice, though, you’ll encounter this issue more often on Windows, mainly because Windows editors are more likely to allow saving files without a trailing newline (or have mixed line-ending settings).

"files.trimTrailingWhitespace": true,
"files.insertFinalNewline": true,
"files.trimFinalNewlines": true,
"files.eol": "\n",
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it really necessary?

Copy link
Contributor

Choose a reason for hiding this comment

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

The formality checks will fail if these are not true, so having a common editor make it easy to adhere to desired style makes sense to me.

If you disagree with the formality checks, that is another discussion (though personally I think that ship has sailed).

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry I don't bother to read the manual.
What I'm asking here is eol specifically - in what conditions it's not \n, on Windows?
And if you know - where is the corresponding formality check?

Copy link
Contributor

@danielfdickinson danielfdickinson Jan 10, 2026

Choose a reason for hiding this comment

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

Ah. Sorry didn't realize was only the eol. I think MacOS is even weirder (\r only or \n\r - I can never remember exactly). I don't know if there is a formality check for this (though it would be useful to have) but things will fail in funky, non-obvious ways if the line-endings are wrong.

Setting Git correctly is the best defence for this, really. If there is a .gitattributes or other Git way to address this at the repo level it would be better.

I'm thinking particularly for newbies who genuinely want to help, or similarly with well-meaning occasional contributors.

[aside: don't read if you're in a bad mood and can't respond well to honesty]

It is unfortunate that the type of contributors who get upset and abusive over non-acceptance or being asked for changes makes regulars grumpy and less willing to help the good ones. And then there is the fact that sometimes it is really not the contributor who is being unreasonable and unpleasant. We are all human, and some grouchier and less helpful than others, regardless of regular contributor status or not.

I personally, whether on a project where I am lead or an occasional get annoyed with 'unpleasant individual' whether they are regulars, new, or occasional.

The "Be nice to each other" rule of OpenWrt seems to not be particularly well practised by some members of the community, and makes the project more hostile than it should be.

The project isn't supposed to be here to make anyone feel superior or entitled, nor to cater to free riders (especially unpleasant ones), or other undesirable attitudes and traits.

In my view it is about collaborating on making something great for all of us.

[/aside]

Copy link
Contributor

Choose a reason for hiding this comment

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

In my impression IDEs usually follow what majority of the project have for eol, so they always auto set it to the right thing whichever OS you are on.
I might be wrong on this.

Copy link
Contributor

Choose a reason for hiding this comment

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

Fair comment, and no I have not. Enforcing EOL editor-side is not something I have needed because I configure git for that. It might be good if @GeorgeSapkin could check for wrong line endings in the formalities, and if contributors are having problems with it, we could revisit.

Copy link
Member Author

Choose a reason for hiding this comment

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

Working on it GeorgeSapkin/hyperstickler#11 💪

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"files.eol": "\n",

Copy link
Contributor

Choose a reason for hiding this comment

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

Or if we want some kind of not necessary needed duplication of the formalities here, let's keep them separated and commented out.

Copy link
Contributor

Choose a reason for hiding this comment

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

files.trimTrailingWhitespace": true is problematic when editing patches. It seemed like a good idea... Now I remember why I didn't have that option still enabled in my editor.

@bam80
Copy link
Contributor

bam80 commented Jan 9, 2026

Thanks for the continuation, see the comments, though.

Copy link
Contributor

@danielfdickinson danielfdickinson 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 for this. It will certainly make it easier to keep in line with the style guidelines.

"files.trimFinalNewlines": true,
"files.eol": "\n",
"git.inputValidation": true,
"git.inputValidationSubjectLength": 60,
Copy link
Contributor

Choose a reason for hiding this comment

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

@GeorgeSapkin I thought the formality check was for 50 characters. Am I mistaken?

Copy link
Member

Choose a reason for hiding this comment

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

There's a soft limit of 50 and a hard limit of 60. Soft limit results in a warning, hard - in a fail.

@BKPepe
Copy link
Member Author

BKPepe commented Jan 10, 2026

Wow, this discussion really took off—I wasn't expecting that! :) Apologies for the delayed reply; I think we're dealing with a time zone difference here.

Anyway, does it look like everything is resolved now? Are we ready to merge, or is there anything left to discuss?

@danielfdickinson
Copy link
Contributor

It obviously LGTM, but hopefully others will chime in with an ACK too.

@hnyman hnyman merged commit d1acb1c into openwrt:master Mar 13, 2026
16 of 18 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.

6 participants