Re: Add --check option to pgindent

On Tue Dec 12, 2023 at 5:47 AM CST, Euler Taveira wrote:
> On Tue, Dec 12, 2023, at 7:28 AM, Michael Banck wrote:
> > On Tue, Dec 12, 2023 at 11:18:59AM +0100, Daniel Gustafsson wrote:
> > > > On 12 Dec 2023, at 01:09, Tristan Partin <tristan@neon.tech> wrote:
> > > >
> > > > Not sold on the name, but --check is a combination of --silent-diff
> > > > and --show-diff. I envision --check mostly being used in CI
> > > > environments. I recently came across a situation where this behavior
> > > > would have been useful. Without --check, you're left to capture the
> > > > output of --show-diff and exit 2 if the output isn't empty by
> > > > yourself.
> > >
> > > I wonder if we should model this around the semantics of git diff to
> > > keep it similar to other CI jobs which often use git diff?  git diff
> > > --check means "are there conflicts or issues" which isn't really
> > > comparable to here, git diff --exit-code however is pretty much
> > > exactly what this is trying to accomplish.
> > >
> > > That would make pgindent --show-diff --exit-code exit with 1 if there
> > > were diffs and 0 if there are no diffs.
> >
> > To be honest, I find that rather convoluted; contrary to "git diff", I
> > believe the primary action of pgident is not to show diffs, so I find
> > the proposed --check option to be entirely reasonable from a UX
> > perspective.
> >
> > On the other hand, tying a "does this need re-indenting?" question to a
> > "--show-diff --exit-code" option combination is not very obvious (to me,
> > at least).
>
> Multiple options to accomplish a use case might not be obvious. I'm wondering
> if we can combine it into a unique option.
>
> --show-diff             show the changes that would be made
> --silent-diff           exit with status 2 if any changes would be made
> + --check                 combination of --show-diff and --silent-diff
>
> I mean
>
> --diff=show,silent,check
>
> When you add exceptions, it starts to complicate the UI.
>
> usage("Cannot have both --silent-diff and --show-diff")
>    if $silent_diff && $show_diff;
>
> +usage("Cannot have both --check and --show-diff")
> +  if $check && $show_diff;
> +
> +usage("Cannot have both --check and --silent-diff")
> +  if $check && $silent_diff;
> +

I definitely agree. I just didn't want to remove options, but if people
are ok with that, I will just change the interface.

I like the git-diff semantics or have --diff and --check, similar to how
Python's black[0] is.

[0]: https://github.com/psf/black

--
Tristan Partin
Neon (https://neon.tech)