Horton

Thread

Commits

  1. Add script to keep .editorconfig in sync with .gitattributes

  2. Remove non-existing file from .gitattributes

  1. Add trim_trailing_whitespace to editorconfig file

    Jelte Fennema-Nio <postgres@jeltef.nl> — 2024-02-14T16:35:13Z

    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.

  2. Re: Add trim_trailing_whitespace to editorconfig file

    Melanie Plageman <melanieplageman@gmail.com> — 2024-02-14T22:06:35Z

    On Wed, Feb 14, 2024 at 11:35 AM Jelte Fennema-Nio <postgres@jeltef.nl> 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

  3. Re: Add trim_trailing_whitespace to editorconfig file

    Daniel Gustafsson <daniel@yesql.se> — 2024-02-14T22:19:55Z

    > On 14 Feb 2024, at 23:06, Melanie Plageman <melanieplageman@gmail.com> wrote:
> 
> On Wed, Feb 14, 2024 at 11:35 AM Jelte Fennema-Nio <postgres@jeltef.nl> 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

  4. Re: Add trim_trailing_whitespace to editorconfig file

    Jelte Fennema-Nio <postgres@jeltef.nl> — 2024-02-15T09:26:34Z

    On Wed, 14 Feb 2024 at 23:19, Daniel Gustafsson <daniel@yesql.se> 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.

  5. Re: Add trim_trailing_whitespace to editorconfig file

    Peter Eisentraut <peter@eisentraut.org> — 2024-02-15T15:57:05Z

    On 15.02.24 10:26, Jelte Fennema-Nio wrote:
> On Wed, 14 Feb 2024 at 23:19, Daniel Gustafsson <daniel@yesql.se> 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.

  6. Re: Add trim_trailing_whitespace to editorconfig file

    Jelte Fennema-Nio <postgres@jeltef.nl> — 2024-02-15T17:47:45Z

    On Thu, 15 Feb 2024 at 16:57, Peter Eisentraut <peter@eisentraut.org> 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.

  7. Re: Add trim_trailing_whitespace to editorconfig file

    Peter Eisentraut <peter@eisentraut.org> — 2024-02-16T10:44:56Z

    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.

  8. Re: Add trim_trailing_whitespace to editorconfig file

    Jelte Fennema-Nio <postgres@jeltef.nl> — 2024-02-19T15:21:27Z

    On Fri, 16 Feb 2024 at 11:45, Peter Eisentraut <peter@eisentraut.org> 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.

  9. Re: Add trim_trailing_whitespace to editorconfig file

    Peter Eisentraut <peter@eisentraut.org> — 2024-04-04T13:25:21Z

    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?

  10. Re: Add trim_trailing_whitespace to editorconfig file

    Jelte Fennema-Nio <postgres@jeltef.nl> — 2024-04-04T14:58:25Z

    On Thu, 4 Apr 2024 at 15:25, Peter Eisentraut <peter@eisentraut.org> 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.

  11. Re: Add trim_trailing_whitespace to editorconfig file

    Peter Eisentraut <peter@eisentraut.org> — 2024-04-04T15:23:28Z

    On 04.04.24 16:58, Jelte Fennema-Nio wrote:
> On Thu, 4 Apr 2024 at 15:25, Peter Eisentraut<peter@eisentraut.org>  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.

  12. Re: Add trim_trailing_whitespace to editorconfig file

    Jelte Fennema-Nio <postgres@jeltef.nl> — 2024-04-04T15:28:25Z

    On Thu, 4 Apr 2024 at 17:23, Peter Eisentraut <peter@eisentraut.org> 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 <peter@eisentraut.org> wrote:
>
> On 04.04.24 16:58, Jelte Fennema-Nio wrote:
> > On Thu, 4 Apr 2024 at 15:25, Peter Eisentraut<peter@eisentraut.org>  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.
>

  13. Re: Add trim_trailing_whitespace to editorconfig file

    Jelte Fennema-Nio <postgres@jeltef.nl> — 2024-04-09T10:42:22Z

    On Thu, 4 Apr 2024 at 16:58, Jelte Fennema-Nio <postgres@jeltef.nl> 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.

  14. Re: Add trim_trailing_whitespace to editorconfig file

    Jelte Fennema-Nio <postgres@jeltef.nl> — 2024-08-07T17:09:59Z

    On Tue, 9 Apr 2024 at 12:42, Jelte Fennema-Nio <postgres@jeltef.nl> wrote:
> 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.

I would love to see this patch merged (or at least some feedback on
the latest version). I think it's pretty trivial and really low risk
of breaking anyone's workflow, and it would *significantly* improve my
own workflow.

Matthias mentioned on Discord that our vendored in pg_bsd_indent uses
a tabwidth of 8 and that was showing up ugly in his editor. I updated
the patch to include a fix for that too.

  15. Re: Add trim_trailing_whitespace to editorconfig file

    Andrew Dunstan <andrew@dunslane.net> — 2024-08-07T19:09:51Z

    On 2024-08-07 We 1:09 PM, Jelte Fennema-Nio wrote:
> On Tue, 9 Apr 2024 at 12:42, Jelte Fennema-Nio <postgres@jeltef.nl> wrote:
>> 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.
> I would love to see this patch merged (or at least some feedback on
> the latest version). I think it's pretty trivial and really low risk
> of breaking anyone's workflow, and it would *significantly* improve my
> own workflow.
>
> Matthias mentioned on Discord that our vendored in pg_bsd_indent uses
> a tabwidth of 8 and that was showing up ugly in his editor. I updated
> the patch to include a fix for that too.


You're not meant to use our pg_bsd_indent on its own without the 
appropriate flags, namely (from src/tools/pgindent/pgindent):

"-bad -bap -bbb -bc -bl -cli1 -cp33 -cdb -nce -d0 -di12 -nfc1 -i4 -l79 
-lp -lpl -nip -npro -sac -tpg -ts4"

If that's inconvenient you can create a .indent.pro with the settings.


Also, why are you proposing to undet indent-style for .pl and .pm files? 
That's not in accordance with our perltidy settings 
(src/tools/pgindent/perltidyrc), unless I'm misunderstanding.


cheers


andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

  16. Re: Add trim_trailing_whitespace to editorconfig file

    Jelte Fennema-Nio <postgres@jeltef.nl> — 2024-08-07T20:42:08Z

    On Wed, 7 Aug 2024 at 21:09, Andrew Dunstan <andrew@dunslane.net> wrote:
> You're not meant to use our pg_bsd_indent on its own without the
> appropriate flags, namely (from src/tools/pgindent/pgindent):

Ah sorry, I wasn't clear in what I meant then. I meant that if you
look at the sources of pg_bsd_indent (such as
src/tools/pg_bsd_indent/io.c) then you'll realize that comments are
alligned using tabs of width 8, not tabs of width 4. And right now
.editorconfig configures editors to show all .c files with a tab_width
of 4, because we use that for Postgres source files. The bottom
.gitattributes line now results in an editorconfig rule that sets a
tab_width of 8 for just the c and h files in src/tools/pg_bsd_indent
directory.

> Also, why are you proposing to undet indent-style for .pl and .pm files?
> That's not in accordance with our perltidy settings
> (src/tools/pgindent/perltidyrc), unless I'm misunderstanding.

All the way at the bottom of the .editorconfig file those "ident_style
= unset" lines are overridden to be "tab" for .pl and .pm files.
There's a comment there explaining why it's done that way.

# We want editors to use tabs for indenting Perl files, but we cannot add it
# such a rule to .gitattributes, because certain lines are still indented with
# spaces (e.g. SYNOPSIS blocks).
[*.{pl,pm}]
indent_style = tab

But now thinking about this again after your comment, I realize it's
just as easy and effective to change the script slightly to hardcode
the indent_style for "*.pl" and "*.pm" so that the resulting
.editorconfig looks less confusing. Attached is a patch that does
that.

I also added a .gitattributes rule for .py files, and changed the
default tab_width to unset. Because I realized the resulting
.editorconfig was using tab_width 8 for python files when editing
src/tools/generate_editorconfig.py

  17. Re: Add trim_trailing_whitespace to editorconfig file

    Andrew Dunstan <andrew@dunslane.net> — 2024-08-08T11:27:51Z

    On 2024-08-07 We 4:42 PM, Jelte Fennema-Nio wrote:
> On Wed, 7 Aug 2024 at 21:09, Andrew Dunstan<andrew@dunslane.net>  wrote:
>> You're not meant to use our pg_bsd_indent on its own without the
>> appropriate flags, namely (from src/tools/pgindent/pgindent):
> Ah sorry, I wasn't clear in what I meant then. I meant that if you
> look at the sources of pg_bsd_indent (such as
> src/tools/pg_bsd_indent/io.c) then you'll realize that comments are
> alligned using tabs of width 8, not tabs of width 4. And right now
> .editorconfig configures editors to show all .c files with a tab_width
> of 4, because we use that for Postgres source files. The bottom
> .gitattributes line now results in an editorconfig rule that sets a
> tab_width of 8 for just the c and h files in src/tools/pg_bsd_indent
> directory.


Ah, OK. Yeah, that makes sense.


>
>> Also, why are you proposing to undet indent-style for .pl and .pm files?
>> That's not in accordance with our perltidy settings
>> (src/tools/pgindent/perltidyrc), unless I'm misunderstanding.
> All the way at the bottom of the .editorconfig file those "ident_style
> = unset" lines are overridden to be "tab" for .pl and .pm files.
> There's a comment there explaining why it's done that way.
>
> # We want editors to use tabs for indenting Perl files, but we cannot add it
> # such a rule to .gitattributes, because certain lines are still indented with
> # spaces (e.g. SYNOPSIS blocks).
> [*.{pl,pm}]
> indent_style = tab
>
> But now thinking about this again after your comment, I realize it's
> just as easy and effective to change the script slightly to hardcode
> the indent_style for "*.pl" and "*.pm" so that the resulting
> .editorconfig looks less confusing. Attached is a patch that does
> that.


OK, good, thanks.


>
> I also added a .gitattributes rule for .py files, and changed the
> default tab_width to unset. Because I realized the resulting
> .editorconfig was using tab_width 8 for python files when editing
> src/tools/generate_editorconfig.py


sounds good.

cheers

andrew

--
Andrew Dunstan
EDB:https://www.enterprisedb.com

  18. Re: Add trim_trailing_whitespace to editorconfig file

    Peter Eisentraut <peter@eisentraut.org> — 2024-08-09T13:16:11Z

    On 07.08.24 22:42, Jelte Fennema-Nio wrote:
> I also added a .gitattributes rule for .py files, and changed the
> default tab_width to unset. Because I realized the resulting
> .editorconfig was using tab_width 8 for python files when editing
> src/tools/generate_editorconfig.py

This looks kind of weird:

-*.sgml		whitespace=space-before-tab,trailing-space,tab-in-indent
-*.x[ms]l	whitespace=space-before-tab,trailing-space,tab-in-indent
+*.py		whitespace=space-before-tab,trailing-space,tab-in-indent,tabwidth=4
+*.sgml		whitespace=space-before-tab,trailing-space,tab-in-indent,tabwidth=1
+*.xml		whitespace=space-before-tab,trailing-space,tab-in-indent,tabwidth=1
+*.xsl		whitespace=space-before-tab,trailing-space,tab-in-indent,tabwidth=2

Why add tabwidth settings to files that are not supposed to contain tabs?

  19. Re: Add trim_trailing_whitespace to editorconfig file

    Jelte Fennema-Nio <postgres@jeltef.nl> — 2024-08-09T14:09:59Z

    On Fri, 9 Aug 2024 at 15:16, Peter Eisentraut <peter@eisentraut.org> wrote:
> -*.sgml         whitespace=space-before-tab,trailing-space,tab-in-indent
> -*.x[ms]l       whitespace=space-before-tab,trailing-space,tab-in-indent
> +*.py           whitespace=space-before-tab,trailing-space,tab-in-indent,tabwidth=4
> +*.sgml         whitespace=space-before-tab,trailing-space,tab-in-indent,tabwidth=1
> +*.xml          whitespace=space-before-tab,trailing-space,tab-in-indent,tabwidth=1
> +*.xsl          whitespace=space-before-tab,trailing-space,tab-in-indent,tabwidth=2
>
> Why add tabwidth settings to files that are not supposed to contain tabs?

That's there so that the generated .editorconfig file the correct
indent_size. I guess another approach would be to change the
generate_editorconfig.py script to include hardcoded values for these
4 filetypes.

  20. Re: Add trim_trailing_whitespace to editorconfig file

    Jelte Fennema-Nio <postgres@jeltef.nl> — 2024-09-05T21:28:21Z

    On Fri, 9 Aug 2024 at 16:09, Jelte Fennema-Nio <postgres@jeltef.nl> wrote:
> That's there so that the generated .editorconfig file the correct
> indent_size. I guess another approach would be to change the
> generate_editorconfig.py script to include hardcoded values for these
> 4 filetypes.

Okay, I've done this now. Any chance this can "just" be committed now?
Having to remove trailing spaces by hand whenever I edit sgml files is a
continuous annoyance to me when working on other patches.

  21. Re: Add trim_trailing_whitespace to editorconfig file

    Jelte Fennema-Nio <postgres@jeltef.nl> — 2024-10-07T20:21:19Z

    On Thu, 5 Sept 2024 at 23:28, Jelte Fennema-Nio <postgres@jeltef.nl> wrote:
> Okay, I've done this now.

Is this blocked on anything? I feel it's ready to merge at this point.
I'd really like to not have this problem with trailing whitespace in
sgml files anymore.

  22. Re: Add trim_trailing_whitespace to editorconfig file

    Jelte Fennema-Nio <postgres@jeltef.nl> — 2024-12-25T23:52:38Z

    On Mon, 7 Oct 2024 at 22:21, Jelte Fennema-Nio <postgres@jeltef.nl> wrote:
> Is this blocked on anything? I feel it's ready to merge at this point.
> I'd really like to not have this problem with trailing whitespace in
> sgml files anymore.

I noticed Peter added some additional rules to .gitattributes last
month. So I reran the script that's part of this patch for an up to
date .editorconfig file. See attached.

  23. Re: Add trim_trailing_whitespace to editorconfig file

    Peter Eisentraut <peter@eisentraut.org> — 2025-02-01T09:11:55Z

    On 26.12.24 00:52, Jelte Fennema-Nio wrote:
> On Mon, 7 Oct 2024 at 22:21, Jelte Fennema-Nio <postgres@jeltef.nl> wrote:
>> Is this blocked on anything? I feel it's ready to merge at this point.
>> I'd really like to not have this problem with trailing whitespace in
>> sgml files anymore.
> 
> I noticed Peter added some additional rules to .gitattributes last
> month. So I reran the script that's part of this patch for an up to
> date .editorconfig file. See attached.

Committed.  (I did some small tweaks to the script to satisfy 
pycodestyle more (except the long lines warnings).)