All of lore.kernel.org
 help / color / mirror / Atom feed
From: Felipe Contreras <felipe.contreras@gmail.com>
To: Sergey Organov <sorganov@gmail.com>,
	Felipe Contreras <felipe.contreras@gmail.com>
Cc: git@vger.kernel.org, Matthieu Moy <matthieu.moy@univ-lyon1.fr>
Subject: Re: Can we clarify the purpose of `git diff -s`?
Date: Thu, 11 May 2023 17:28:21 -0600	[thread overview]
Message-ID: <645d7a153ac66_1ba85a29451@chronos.notmuch> (raw)
In-Reply-To: <87fs82zdny.fsf@osv.gnss.ru>

Sergey Organov wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
> > Sergey Organov wrote:
> >> Felipe Contreras <felipe.contreras@gmail.com> writes:
> >> > Sergey Organov wrote:
> >> >> Felipe Contreras <felipe.contreras@gmail.com> writes:
> >> >> > Sergey Organov wrote:
> >> >> >> Felipe Contreras <felipe.contreras@gmail.com> writes:
> >> >> >> > Sergey Organov wrote:
> >> >> >> >
> >> >> >> >> I'd rather think about generic interface for setting/clearing
> >> >> >> >> (multiple) bits through CI than resorting to such convenience
> >> >> >> >> tricks. Once that is in place, one will be able to say "I need these
> >> >> >> >> bits only", "I need to turn these bit(s) on", and "I need to turn
> >> >> >> >> these bit(s) off" conveniently and universally in any part of Git CI
> >> >> >> >> where it's needed.
> >> >> >> >
> >> >> >> > It's possible to achieve both.
> >> >> >> >
> >> >> >> > Imagine your ideal explicit interface. In that interface the default
> >> >> >> > is no output, so you *have* to specify all the bits, for example:
> >> >> >> >
> >> >> >> >   git show --patch
> >> >> >> 
> >> >> >> No, that's not what I meant. There is no point in making "git show" to
> >> >> >> have no output by default, please see below.
> >> >> >> 
> >> >> >> >
> >> >> >> > Or:
> >> >> >> >
> >> >> >> >   git show --raw
> >> >> >> >
> >> >> >> > In this ideal interface it's clear what the user wants to do, because
> >> >> >> > it's explicit.
> >> >> >> >
> >> >> >> >   git show --patch --raw --no-patch
> >> >> >> >
> >> >> >> > Agreed?
> >> >> >> >
> >> >> >> > My proposal achieves your ideal explicit interface, except when no
> >> >> >> > format is specified (e.g. `git show`), a default format is chosen for
> >> >> >> > the user, but that's *only* if the user hasn't specified any format.
> >> >> >> 
> >> >> >> My point is that the default format should be selected as if it has been
> >> >> >> provided by existing options, rather than by some magic hidden in the
> >> >> >> code.
> >> >> >
> >> >> > But why?
> >> >> >
> >> >> > I don't see any benefit, only drawbacks.
> >> >> >
> >> >> >> > If you explicitely specify the output format that you want, then the
> >> >> >> > default is irrelevant to you, thus you have your ideal explicit
> >> >> >> > interface.
> >> >> >> 
> >> >> >> That's not what I had in mind, sorry. It'd rather be something like:
> >> >> >> 
> >> >> >>   --raw: set "raw" bit and clear all the rest
> >> >> >>   --+raw set "raw" bit  (== current --raw)
> >> >> >>   ---raw clear "raw" bit (== --no-raw)
> >> >> >> 
> >> >> >> In this model
> >> >> >> 
> >> >> >>   git show
> >> >> >> 
> >> >> >> would be just an alias for
> >> >> >> 
> >> >> >>   git log -n1 --patch --cc
> >> >> >> 
> >> >> >> and no support for a separate command would be need in the first place.
> >> >> >> 
> >> >> >>   git show --raw
> >> >> >> 
> >> >> >> would then produce expected output that makes sense due to the common
> >> >> >> option processing rules, not because somebody had implemented some
> >> >> >> arbitrary "defaults" for the command.
> >> >> >
> >> >> > But now you are at the mercy of those "arbitrary defaults".
> >> >> 
> >> >> No, see below.
> >> >> 
> >> >> >
> >> >> > Let's say those defaults change, and now the default output of `git show` is
> >> >> > `--stat`.
> >> >> >
> >> >> > Now to generate the same output you have to do:
> >> >> >
> >> >> >   git show --raw
> >> >> >
> >> >> > in one version of git, and:
> >> >> >
> >> >> >   git show --no-stat --patch --raw
> >> >> >
> >> >> > in another.
> >> >> 
> >> >> No: --raw in my model clears all the flags but --raw, so
> >> >> 
> >> >>   git show --raw
> >> >> 
> >> >> will produce exactly the same result: raw output only.
> >> >
> >> > But that {--,--+,---} notion doesn't exist, and I think it's safe to say it
> >> > will never exist. So, could we limit or solution-space to those solutions that
> >> > could have the potential to be merged?
> >> 
> >> I didn't expect it to exist any time soon, just showed a different way
> >> of options design.
> >> 
> >> >
> >> > What you suggest could be easily achieved with:
> >> >
> >> >   git show --silent --raw
> >> >
> >> > But because no other format is explicitely specified, following my notion of
> >> > defaults, that's the same as:
> >> 
> >> The problem that I tried to fight is this notion of defaults that is
> >> somewhat special, so, if I allow for it, the rest of my suggestions
> >> becomes pointless,
> >
> > No, they don't, all you need to do is specify the default explicitely.
> >
> >> and without the "defaults" with non-trivial behavior[*]
> >> 
> >>    git show --raw
> >> 
> >> won't work as expected provided --raw still just sets "raw" bit and
> >> doesn't clear all the rest.
> >
> > It works perfectly fine. There are no bits to clear, because there are no bits
> > set.
> 
> When I set default value to a variable in C, it does have bits set, and
> they are kept unless overwritten, so they are set by default as well.
> Exactly the bits that I've set. Here I've proposed the same principle
> for handling of options.
> 
> What you have in mind is exactly the current behavior

No, it's very different.

 cur: git diff --raw --no-patch # no output
 new: git diff --raw --no-patch # raw output

 cur: git diff -s --raw # no output
 new: git diff -s --raw # raw output

 cur: git diff -s --patch --raw --no-patch # no output
 new: git diff -s --patch --raw --no-patch # raw output

I've no idea what makes you think these are exactly the same.

> > That's the whole point of defaults: you don't have to use them. If you don't
> > like the notion of defaults, then don't use them.
> 
> Once again, the defaults in this form seem to be not needed to me. I
> already got it that you like them, and it looks like we need to agree to
> disagree.

What I (or anyone) think of the defaults is irrelevant. You don't have to use
them.

> > If you specify *any* format option, then the defaults are ignored and no bits
> > are set other than the ones that you explicitly specified.
> 
> That's exactly how it works now,

No, it's not.

Right now the code cannot distinguish between `git diff` and `git diff
--no-patch`, which is precisely why the code can't turn off the
DIFF_FORMAT_PATCH field.

> >> [*] Defaults with trivial behavior is just initializing of internal
> >> variable holding flags with specific value, that is exactly the same as
> >> putting corresponding option(s) at the beginning.
> >
> > Those are not default arguments, those are initial arguments. In many cases
> > they behave the same, but not all.
> 
> In my model they are both. When you set a bit initially, it's then on by
> default.
> 
> In your model all initial bits are effectively cleared just before any
> bit is changed by option, and this is an additional rule. What I'm
> trying to explain is that this additional rule is not needed,

I know it's not needed, but it's better for the end-user, as it doesn't require
any mental baggage.

> as all the functionality could be achieved without it.

All the functionality can be achieved with it as well.

"It can be done" is not an argument in favor of a change, the question is not
if it _can_ be done, the question is if it *should*.

> From all the correct solutions of a problem the simplest one is the
> best.

All absolutist claims are wrong...

* almost all

I think I'll have to send the patches implementing everything so you can see
how my proposal is different from the status quo, not just with code, but with
examples.

Cheers.

-- 
Felipe Contreras

  reply	other threads:[~2023-05-11 23:28 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-11  3:14 Can we clarify the purpose of `git diff -s`? Felipe Contreras
2023-05-11 11:59 ` Sergey Organov
2023-05-11 16:26   ` Junio C Hamano
2023-05-11 17:37   ` Junio C Hamano
2023-05-11 18:04     ` Sergey Organov
2023-05-11 18:27       ` Junio C Hamano
2023-05-11 18:36       ` Felipe Contreras
2023-05-11 18:17     ` Felipe Contreras
2023-05-11 17:41   ` Felipe Contreras
2023-05-11 18:31     ` Sergey Organov
2023-05-11 19:10       ` Felipe Contreras
2023-05-11 19:32         ` Sergey Organov
2023-05-11 19:54           ` Felipe Contreras
2023-05-11 20:24             ` Sergey Organov
2023-05-11 20:59               ` Felipe Contreras
2023-05-11 22:49                 ` Sergey Organov
2023-05-11 23:28                   ` Felipe Contreras [this message]
2023-05-12  8:40                     ` Sergey Organov
2023-05-12 19:19                       ` Felipe Contreras
     [not found]   ` <5bb24e0208dd4a8ca5f6697d578f3ae0@SAMBXP02.univ-lyon1.fr>
2023-05-12  8:15     ` Matthieu Moy
2023-05-12 17:03       ` Junio C Hamano
2023-05-12 18:21         ` Sergey Organov
2023-05-12 19:21           ` Junio C Hamano
2023-05-12 19:34             ` Junio C Hamano
2023-05-12 20:28             ` Felipe Contreras
2023-05-12 20:47               ` Junio C Hamano
2023-05-12 21:01                 ` Felipe Contreras
2023-05-12 21:47                   ` Junio C Hamano
2023-05-12 21:48                     ` Junio C Hamano
2023-05-12 23:21                     ` Felipe Contreras
2023-05-12 21:41                 ` Sergey Organov
2023-05-12 22:17                   ` Junio C Hamano
2023-05-12 22:47                     ` Sergey Organov
2023-05-12 23:07                   ` Felipe Contreras
2023-05-13 14:58                     ` Philip Oakley
2023-05-13 17:45                       ` Sergey Organov
2023-05-12 19:47           ` Felipe Contreras
2023-05-12 19:34         ` Felipe Contreras
2023-05-12 19:17       ` Felipe Contreras

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=645d7a153ac66_1ba85a29451@chronos.notmuch \
    --to=felipe.contreras@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=matthieu.moy@univ-lyon1.fr \
    --cc=sorganov@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.