Re: Add trim_trailing_whitespace to editorconfig file
On Thu, 4 Apr 2024 at 16:58, Jelte Fennema-Nio wrote: > > ISTM that with a small shell script, .editorconfig could be generated > > from .gitattributes? > > Honestly, I don't think building such automation is worth the effort. Okay, I spent the time to add a script to generate the editorconfig based on .gitattributes after all. So attached is a patch that adds that. v5-0001-Add-script-to-keep-.editorconfig-in-sync-with-.gi.patch Description: Binary data
Re: Add trim_trailing_whitespace to editorconfig file
On Thu, 4 Apr 2024 at 17:23, Peter Eisentraut wrote: > git diff-tree --check $(git hash-object -t tree /dev/null) HEAD > > That's what I was hoping for for editorconfig-check, but as I said, the > experience wasn't good. Ah, I wasn't able to find that git incantation. I definitely think it would be good if there was an official cli tool like that for editorconfig, but the Javascript one was the closest I could find. The Go one I haven't tried. On Thu, 4 Apr 2024 at 17:23, Peter Eisentraut wrote: > > On 04.04.24 16:58, Jelte Fennema-Nio wrote: > > On Thu, 4 Apr 2024 at 15:25, Peter Eisentraut wrote: > >> Everybody has git. Everybody who edits .gitattributes can use git to > >> check what they did. > > What CLI command do you use to fix/ gitattributes on all existing > > files? Afaict there's no command to actually remove the trailing > > whitespace that git add complains about. If you don't have such a > > command, then afaict updating gitattributes is also essentially > > blind-updating. > > I don't have a command to fix files automatically, but I have a command > to check them: > > git diff-tree --check $(git hash-object -t tree /dev/null) HEAD > > That's what I was hoping for for editorconfig-check, but as I said, the > experience wasn't good. >
Re: Add trim_trailing_whitespace to editorconfig file
On 04.04.24 16:58, Jelte Fennema-Nio wrote: On Thu, 4 Apr 2024 at 15:25, Peter Eisentraut wrote: Everybody has git. Everybody who edits .gitattributes can use git to check what they did. What CLI command do you use to fix/ gitattributes on all existing files? Afaict there's no command to actually remove the trailing whitespace that git add complains about. If you don't have such a command, then afaict updating gitattributes is also essentially blind-updating. I don't have a command to fix files automatically, but I have a command to check them: git diff-tree --check $(git hash-object -t tree /dev/null) HEAD That's what I was hoping for for editorconfig-check, but as I said, the experience wasn't good.
Re: Add trim_trailing_whitespace to editorconfig file
On Thu, 4 Apr 2024 at 15:25, Peter Eisentraut wrote: > Everybody has git. Everybody who edits .gitattributes can use git to > check what they did. What CLI command do you use to fix/ gitattributes on all existing files? Afaict there's no command to actually remove the trailing whitespace that git add complains about. If you don't have such a command, then afaict updating gitattributes is also essentially blind-updating. > But I don't want users of a common tool to bear the > burden of blindly updating files for a much-less-common tool. It's used quite a bit. Many editors/IDEs have built in support (Vim, Visual Studio, IntelliJ), and the ones that don't have an easy to install plugin. It's not meant to be used as a command line tool, but as the name suggests it's meant as editor integration. > ISTM that with a small shell script, .editorconfig could be generated > from .gitattributes? Honestly, I don't think building such automation is worth the effort. Changing the .editorconfig file to be the same is pretty trivial if you look at the existing examples, honestly editorconfig syntax is much more straightforward to me than the gitattributes one. Also gitattributes is only changed very rarely, only 15 times in the 10 years since its creation in our repo, which makes any automation around it probably not worth the investement. This whole comment really seems to only really be about 0004. We already have an outdated editorconfig file in the repo, and it's severely annoying me whenever I'm writing any docs for postgres because it doesn't trim my trailing spaces. If we wouldn't have this editorconfig file in the repo at all, it would actually be better for me, because I could maintain my own file locally myself. But now because there's an incorrect file, I'd have to git stash/pop all the time. Is there any chance the other commits can be at least merged.
Re: Add trim_trailing_whitespace to editorconfig file
On 19.02.24 16:21, Jelte Fennema-Nio wrote: v3-0003-Bring-editorconfig-in-line-with-gitattributes.patch I question whether we need to add rules to .editorconfig about files that are generated or imported from elsewhere, since those are not meant to be edited. I agree that it's not strictly necessary to have .editorconfig match .gitattributes for files that are not meant to be edited by hand. But I don't really see a huge downside either, apart from having a few extra lines it .editorconfig. And adding these lines does have a few benefits: 1. It makes it easy to ensure that .editorconfig and .gitattributes stay in sync 2. If someone opens a file that they are not supposed to edit by hand, and then saves it. Then no changes are made. As opposed to suddenly making some whitespace changes Attached is a new patchset with the first commit split in three separate commits, which configure: 1. Files meant to be edited by hand) 2. Output test files (maybe edited by hand) 3. Imported/autogenerated files > diff --git a/.gitattributes b/.gitattributes > index e9ff4a56bd..7923fc3387 100644 > --- a/.gitattributes > +++ b/.gitattributes > @@ -1,3 +1,4 @@ > +# IMPORTANT: When updating this file, also update .editorconfig to match. Everybody has git. Everybody who edits .gitattributes can use git to check what they did. Not everybody has editorconfig-related tools. I tried the editorconfig-checker that you had mentioned (I tried the Go version, not the JavaScript one, because the former is packaged for Homebrew and Debian), but it was terrible and unusable. Maybe I'm holding it wrong. But I don't want users of a common tool to bear the burden of blindly updating files for a much-less-common tool. This is how we got years of blindly updating Windows build files. The result will be to that people will instead avoid updating .gitattributes. ISTM that with a small shell script, .editorconfig could be generated from .gitattributes?
Re: Add trim_trailing_whitespace to editorconfig file
On Fri, 16 Feb 2024 at 11:45, Peter Eisentraut wrote:
> I have committed that one.
Thanks :)
> v3-0002-Require-final-newline-in-.po-files.patch
>
> The .po files are imported from elsewhere, so I'm not sure this is going
> to have the desired effect. Perhaps it's worth cleaning up, but it
> would require more steps.
Okay, yeah that would need to be changed at the source then. Removed
this change from the newly attached patchset, as well as updating
editorconfig to have "insert_final_newline = unset" for .po files.
> v3-0003-Bring-editorconfig-in-line-with-gitattributes.patch
>
> I question whether we need to add rules to .editorconfig about files
> that are generated or imported from elsewhere, since those are not meant
> to be edited.
I agree that it's not strictly necessary to have .editorconfig match
.gitattributes for files that are not meant to be edited by hand. But
I don't really see a huge downside either, apart from having a few
extra lines it .editorconfig. And adding these lines does have a few
benefits:
1. It makes it easy to ensure that .editorconfig and .gitattributes stay in sync
2. If someone opens a file that they are not supposed to edit by hand,
and then saves it. Then no changes are made. As opposed to suddenly
making some whitespace changes
Attached is a new patchset with the first commit split in three
separate commits, which configure:
1. Files meant to be edited by hand)
2. Output test files (maybe edited by hand)
3. Imported/autogenerated files
The first one is definitely the most useful to me personally.
From 419d2d0e45d40a4cddb942fbc63c3c54f4dd479d Mon Sep 17 00:00:00 2001
From: Jelte Fennema-Nio
Date: Mon, 19 Feb 2024 16:03:31 +0100
Subject: [PATCH v4 2/5] Include test output files in .editorconfig
Most of the time these will be copy pasted from a test run, but if
there's a trivial change someone might want to make the change by hand.
By adding them to .editorconfig, we make sure that doing so won't mess
up the formatting.
---
.editorconfig | 9 +
1 file changed, 9 insertions(+)
diff --git a/.editorconfig b/.editorconfig
index 0e7c2048f9..92a16bd5de 100644
--- a/.editorconfig
+++ b/.editorconfig
@@ -27,3 +27,12 @@ trim_trailing_whitespace = false
[src/backend/catalog/sql_features.txt]
trim_trailing_whitespace = false
+
+# Test output files that contain extra whitespace
+[*.out]
+trim_trailing_whitespace = false
+insert_final_newline = unset
+
+[src/interfaces/ecpg/test/expected/*]
+trim_trailing_whitespace = false
+insert_final_newline = unset
--
2.34.1
From b05173b619c45d9ad859da50bfa78d1fc626fe3d Mon Sep 17 00:00:00 2001
From: Jelte Fennema-Nio
Date: Wed, 14 Feb 2024 17:19:42 +0100
Subject: [PATCH v4 1/5] Handle blank-at-eof/blank-at-eol in .editorconfig too
For many files in the repo our .gitattributes file is configured to
complain about trailing whitespace at the end of a line, as well as not
including a final newline at the end of the file. This updates our
.editorconfig file, to make editors and IDEs fix these issues
automatically on save in the same way for files that are intended to be
edited by hand.
---
.editorconfig | 15 +++
1 file changed, 15 insertions(+)
diff --git a/.editorconfig b/.editorconfig
index d69a3d1dc4..0e7c2048f9 100644
--- a/.editorconfig
+++ b/.editorconfig
@@ -1,5 +1,9 @@
root = true
+[*]
+trim_trailing_whitespace = true
+insert_final_newline = true
+
[*.{c,h,l,y,pl,pm}]
indent_style = tab
indent_size = tab
@@ -12,3 +16,14 @@ indent_size = 1
[*.xsl]
indent_style = space
indent_size = 2
+
+# Certain data files that contain special whitespace, and other special cases
+[*.data]
+trim_trailing_whitespace = false
+insert_final_newline = unset
+
+[contrib/pgcrypto/sql/pgp-armor.sql]
+trim_trailing_whitespace = false
+
+[src/backend/catalog/sql_features.txt]
+trim_trailing_whitespace = false
base-commit: e1b7fde418f2c0ba4ab0d9fbfa801ef62d96397b
--
2.34.1
From 97fe71d7769a3e4191adedbfe4092afec696ea7b Mon Sep 17 00:00:00 2001
From: Jelte Fennema-Nio
Date: Thu, 15 Feb 2024 10:23:19 +0100
Subject: [PATCH v4 4/5] Add note about keeping .editorconfig and
.gitattributes in sync
Now that they are in sync, hopefully this reminder makes sure we keep
them that way. Automation to detect them being out of sync seems
excessive.
---
.editorconfig | 1 +
.gitattributes | 1 +
2 files changed, 2 insertions(+)
diff --git a/.editorconfig b/.editorconfig
index c742e0f844..3f5c4e0ef8 100644
--- a/.editorconfig
+++ b/.editorconfig
@@ -1,3 +1,4 @@
+# IMPORTANT: When updating this file, also update .gitattributes to match.
root = true
[*]
diff --git a/.gitattributes b/.gitattributes
index e9ff4a56bd..7923fc3387 100644
--- a/.gitattributes
+++ b/.gitattributes
@@ -1,3 +1,4 @@
+# IMPORTANT: When updating this file, also update .editorconfig to match.
* whitespace=space-before-tab,trailing-space
*.[chly] whitespace=space-before-tab,trailing-space,indent-with-non-tab,tabwidth=4
*.pl
Re: Add trim_trailing_whitespace to editorconfig file
v3-0001-Remove-non-existing-file-from-.gitattributes.patch I have committed that one. v3-0002-Require-final-newline-in-.po-files.patch The .po files are imported from elsewhere, so I'm not sure this is going to have the desired effect. Perhaps it's worth cleaning up, but it would require more steps. v3-0003-Bring-editorconfig-in-line-with-gitattributes.patch I question whether we need to add rules to .editorconfig about files that are generated or imported from elsewhere, since those are not meant to be edited.
Re: Add trim_trailing_whitespace to editorconfig file
On Thu, 15 Feb 2024 at 16:57, Peter Eisentraut wrote: > Is there a command-line tool to verify the syntax of .editorconfig and > check compliance of existing files? > > I'm worried that expanding .editorconfig with detailed per-file rules > will lead to a lot of mistakes and blind editing, if we don't have > verification tooling. I tried this one just now: https://github.com/editorconfig-checker/editorconfig-checker.javascript I fixed all the issues by updating my patchset to use "unset" for insert_final_newline instead of "false". All other files were already clean, which makes sense because the new editorconfig rules are exactly the same as gitattributes (which I'm guessing we are checking in CI/buildfarm). So I don't think it makes sense to introduce another tool to check the same thing again. v3-0002-Require-final-newline-in-.po-files.patch Description: Binary data v3-0004-Add-note-about-keeping-.editorconfig-and-.gitattr.patch Description: Binary data v3-0001-Remove-non-existing-file-from-.gitattributes.patch Description: Binary data v3-0005-Add-indent-information-about-gitattributes-to-edi.patch Description: Binary data v3-0003-Bring-editorconfig-in-line-with-gitattributes.patch Description: Binary data
Re: Add trim_trailing_whitespace to editorconfig file
On 15.02.24 10:26, Jelte Fennema-Nio wrote: On Wed, 14 Feb 2024 at 23:19, Daniel Gustafsson wrote: +1 from me. But when do we want it to be false? That is, why not declare it true for all file types? Regression test .out files commonly have spaces at the end of the line. (Not to mention the ECPG .c files but they probably really shouldn't have.) Attached is v2, which now makes the rules between gitattributes and editorconfig completely identical. As well as improving two minor things about .gitattributes before doing that. Is there a command-line tool to verify the syntax of .editorconfig and check compliance of existing files? I'm worried that expanding .editorconfig with detailed per-file rules will lead to a lot of mistakes and blind editing, if we don't have verification tooling.
Re: Add trim_trailing_whitespace to editorconfig file
On Wed, 14 Feb 2024 at 23:19, Daniel Gustafsson wrote: > > +1 from me. But when do we want it to be false? That is, why not > > declare it true for all file types? > > Regression test .out files commonly have spaces at the end of the line. (Not > to mention the ECPG .c files but they probably really shouldn't have.) Attached is v2, which now makes the rules between gitattributes and editorconfig completely identical. As well as improving two minor things about .gitattributes before doing that. v2-0002-Require-final-newline-in-.po-files.patch Description: Binary data v2-0003-Bring-editorconfig-in-line-with-gitattributes.patch Description: Binary data v2-0004-Add-note-about-keeping-.editorconfig-and-.gitattr.patch Description: Binary data v2-0001-Remove-non-existing-file-from-.gitattributes.patch Description: Binary data
Re: Add trim_trailing_whitespace to editorconfig file
> On 14 Feb 2024, at 23:06, Melanie Plageman wrote: > > On Wed, Feb 14, 2024 at 11:35 AM Jelte Fennema-Nio wrote: >> >> This brings our .gitattributes and .editorconfig files more in line. I >> had the problem that "git add" would complain often about trailing >> whitespaces when I was changing sgml files specifically. > > +1 from me. But when do we want it to be false? That is, why not > declare it true for all file types? Regression test .out files commonly have spaces at the end of the line. (Not to mention the ECPG .c files but they probably really shouldn't have.) -- Daniel Gustafsson
Re: Add trim_trailing_whitespace to editorconfig file
On Wed, Feb 14, 2024 at 11:35 AM Jelte Fennema-Nio wrote: > > This brings our .gitattributes and .editorconfig files more in line. I > had the problem that "git add" would complain often about trailing > whitespaces when I was changing sgml files specifically. +1 from me. But when do we want it to be false? That is, why not declare it true for all file types? - Melanie
Add trim_trailing_whitespace to editorconfig file
This brings our .gitattributes and .editorconfig files more in line. I had the problem that "git add" would complain often about trailing whitespaces when I was changing sgml files specifically. v1-0001-Configure-trailing-whitespace-trimming-in-editorc.patch Description: Binary data
