On Mon, 18 Dec 2023 at 22:18, Tristan Partin <tristan@neon.tech> wrote:
> Here is an additional patch which implements the behavior you described.
> The first patch is just Daniel's squashed version of my patches.
I think we'd still want the early exit behaviour when only --check is
provided. No need to spend time checking more files if you're not
doing anything else.
On Mon, 18 Dec 2023 at 22:34, Tristan Partin <tristan@neon.tech> wrote:
> To me, the two options seem at odds, like you either check or write. How
> would you feel about just capturing the diffs that are printed and
> patch(1)-ing them?
I tried capturing the diffs and patching them, but simply piping the
pgindent output to patch(1) didn't work because the pipe loses the
exit code of pgindent. -o pipefail would help with this, but it's not
available in all shells. Also then it's suddenly unclear if pgident
failed or if patch failed.
Attached are two additional patches (in addition to your unchanged
previous ones). One which adds the --write flag and one which early
exits with --check when neither --write or --diff are provided. The
additional code I added for the --write flag is really minimal IMO.
On Tue, 19 Dec 2023 at 14:51, Andrew Dunstan <andrew@dunslane.net> wrote:
> Not sold on this. I don't want pgindent applied automatically to my
> uncommitted changes, but I do want a reminder that I forgot to run it.
> In any case, as you say it's a different topic.
To be clear, with this patch just passing --check won't apply the
changes automatically. Only when passing both --write and --check will
it write the files.
To clarify my commit workflow is as follows:
git add -p # interactively select all the changes that I want in my commit
git commit
# pre-commit hook fails because of pgindent and "fixes" my files in
place but doesn't add them to the commit yet
git add -p
# I look at all the changes that pgindent make to see if they are
sensible and accept them, if not I find some other way to fix them
git commit
# now commit succeeded
On Tue, 19 Dec 2023 at 14:58, Daniel Gustafsson <daniel@yesql.se> wrote:
> I think we risk making the tool confusing if we add too many options which can
> all be combined to suit every need. The posted v5 seems like a good compromise
> I reckon.
I personally think these options are completely independent. So it's
more confusing to me that they cannot be combined. I updated the help
message in 0003 as well, to describe them as completely independent:
--diff show the changes that need to be made
--check exit with status 2 if any changes need to be made
--write rewrites files that need changes (default
if neither --check/--diff/--write are provided)
PS. prettier (javascript formatter) allows both --check and --write at
the same time to do exactly this
PPS. the help message didn't contain anything about pgindent writing
files by default (i.e. when --check or --diff are not provided)
PPPS. I attached my new pre-commit hook for reference.