Обсуждение: Why --backup-and-modify-in-place in perltidy config?
I did a trial run following the current pgindent README procedure, and
noticed that the perltidy step left me with a pile of '.bak' files
littering the entire tree. This seems like a pretty bad idea because
a naive "git add ." would have committed them. It's evidently because
src/tools/pgindent/perltidyrc includes --backup-and-modify-in-place.
Is there a good reason for that, and if so what is it?
Also, is there a reason why the perltidy invocation command hasn't
been packaged into a shell script, rather than expecting the committer
to copy-and-paste a rather large string?
regards, tom lane
On 08/14/2016 04:38 PM, Tom Lane wrote: > I did a trial run following the current pgindent README procedure, and > noticed that the perltidy step left me with a pile of '.bak' files > littering the entire tree. This seems like a pretty bad idea because > a naive "git add ." would have committed them. It's evidently because > src/tools/pgindent/perltidyrc includes --backup-and-modify-in-place. > Is there a good reason for that, and if so what is it? We should probably specify -bext='/', which would cause the backup files to be deleted unless an error occurred. Alternatively, we could just remove the in-place parameter and write a command that moved the new .tdy files over the original when perltidy was finished. > > Also, is there a reason why the perltidy invocation command hasn't > been packaged into a shell script, rather than expecting the committer > to copy-and-paste a rather large string? No idea. Sounds like a good thing to do. cheers andrew
Andrew Dunstan <andrew@dunslane.net> writes:
> On 08/14/2016 04:38 PM, Tom Lane wrote:
>> I did a trial run following the current pgindent README procedure, and
>> noticed that the perltidy step left me with a pile of '.bak' files
>> littering the entire tree. This seems like a pretty bad idea because
>> a naive "git add ." would have committed them. It's evidently because
>> src/tools/pgindent/perltidyrc includes --backup-and-modify-in-place.
BTW, after experimenting with this, I did not find any way to get perltidy
to overwrite the original files without making a backup file.
> We should probably specify -bext='/', which would cause the backup files
> to be deleted unless an error occurred.
Really? That seems a bit magic, and it's certainly undocumented.
> Alternatively, we could just remove the in-place parameter and write a
> command that moved the new .tdy files over the original when perltidy
> was finished.
I was thinking about just removing all the .bak files afterwards, ie
automating the existing manual process. As long as we're making an
invocation script anyway, that's easy.
regards, tom lane
On 08/15/2016 10:19 AM, Tom Lane wrote: > Andrew Dunstan <andrew@dunslane.net> writes: >> On 08/14/2016 04:38 PM, Tom Lane wrote: >>> I did a trial run following the current pgindent README procedure, and >>> noticed that the perltidy step left me with a pile of '.bak' files >>> littering the entire tree. This seems like a pretty bad idea because >>> a naive "git add ." would have committed them. It's evidently because >>> src/tools/pgindent/perltidyrc includes --backup-and-modify-in-place. > BTW, after experimenting with this, I did not find any way to get perltidy > to overwrite the original files without making a backup file. > >> We should probably specify -bext='/', which would cause the backup files >> to be deleted unless an error occurred. > Really? That seems a bit magic, and it's certainly undocumented. We must be using different versions. > >> Alternatively, we could just remove the in-place parameter and write a >> command that moved the new .tdy files over the original when perltidy >> was finished. > I was thinking about just removing all the .bak files afterwards, ie > automating the existing manual process. As long as we're making an > invocation script anyway, that's easy. > > WFM. cheers andrew
Andrew Dunstan <andrew@dunslane.net> writes:
> On 08/15/2016 10:19 AM, Tom Lane wrote:
>> Andrew Dunstan <andrew@dunslane.net> writes:
>>> We should probably specify -bext='/', which would cause the backup files
>>> to be deleted unless an error occurred.
>> Really? That seems a bit magic, and it's certainly undocumented.
> We must be using different versions.
Hmm ... I'm using the RHEL6 version, which claims to be v20090616,
which is what pgindent/README says to use.
regards, tom lane
On Mon, Aug 15, 2016 at 11:19 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Andrew Dunstan <andrew@dunslane.net> writes: >> On 08/14/2016 04:38 PM, Tom Lane wrote: >>> I did a trial run following the current pgindent README procedure, and >>> noticed that the perltidy step left me with a pile of '.bak' files >>> littering the entire tree. This seems like a pretty bad idea because >>> a naive "git add ." would have committed them. It's evidently because >>> src/tools/pgindent/perltidyrc includes --backup-and-modify-in-place. > > BTW, after experimenting with this, I did not find any way to get perltidy > to overwrite the original files without making a backup file. > >> We should probably specify -bext='/', which would cause the backup files >> to be deleted unless an error occurred. > > Really? That seems a bit magic, and it's certainly undocumented. > >> Alternatively, we could just remove the in-place parameter and write a >> command that moved the new .tdy files over the original when perltidy >> was finished. > > I was thinking about just removing all the .bak files afterwards, ie > automating the existing manual process. As long as we're making an > invocation script anyway, that's easy. The tree does not have any .bak file, and those refer to backup copies normally. Perhaps it would make sense to include those in root's .gitignore? That would save from an unfortunate manipulation of git add in the future. -- Michael
Michael Paquier <michael.paquier@gmail.com> writes:
> The tree does not have any .bak file, and those refer to backup copies
> normally. Perhaps it would make sense to include those in root's
> .gitignore? That would save from an unfortunate manipulation of git
> add in the future.
We've generally refrained from adding things like that to the .gitignore
files. If you've got unexpected trash in your tree, you probably ought
to be told about it. There was some discussion back-when about including
common editor backup extensions and suchlike, but the consensus was that
those are better handled in user-private git config files.
regards, tom lane
On 08/15/2016 02:23 PM, Tom Lane wrote: > Andrew Dunstan <andrew@dunslane.net> writes: >> On 08/15/2016 10:19 AM, Tom Lane wrote: >>> Andrew Dunstan <andrew@dunslane.net> writes: >>>> We should probably specify -bext='/', which would cause the backup files >>>> to be deleted unless an error occurred. >>> Really? That seems a bit magic, and it's certainly undocumented. >> We must be using different versions. > Hmm ... I'm using the RHEL6 version, which claims to be v20090616, > which is what pgindent/README says to use. Ah. I have Fedora 22's v20140711. 2009 seems a bit ancient :-) Anyway, what you've done seems fine. cheers andrew
On 16/08/16 16:19, Andrew Dunstan wrote: > > > On 08/15/2016 02:23 PM, Tom Lane wrote: >> Andrew Dunstan <andrew@dunslane.net> writes: >>> On 08/15/2016 10:19 AM, Tom Lane wrote: >>>> Andrew Dunstan <andrew@dunslane.net> writes: >>>>> We should probably specify -bext='/', which would cause the backup >>>>> files >>>>> to be deleted unless an error occurred. >>>> Really? That seems a bit magic, and it's certainly undocumented. >>> We must be using different versions. >> Hmm ... I'm using the RHEL6 version, which claims to be v20090616, >> which is what pgindent/README says to use. > > > Ah. I have Fedora 22's v20140711. 2009 seems a bit ancient :-) Anyway, > what you've done seems fine. > > cheers > > andrew > > > A vintage year, but for the best taste, it has to have been to be aged on Oak! :-)
On Mon, Aug 15, 2016 at 10:19:12AM -0400, Tom Lane wrote: > Andrew Dunstan <andrew@dunslane.net> writes: > > On 08/14/2016 04:38 PM, Tom Lane wrote: > >> I did a trial run following the current pgindent README procedure, and > >> noticed that the perltidy step left me with a pile of '.bak' files > >> littering the entire tree. This seems like a pretty bad idea because > >> a naive "git add ." would have committed them. It's evidently because > >> src/tools/pgindent/perltidyrc includes --backup-and-modify-in-place. > > BTW, after experimenting with this, I did not find any way to get perltidy > to overwrite the original files without making a backup file. Yep, that's why --backup-and-modify-in-place had to be used. I have a local script to remove file with specified extentions, but didn't document that cleanup step. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + As you are, so once was I. As I am, so you will be. + + Ancient Roman grave inscription +