Git Mailing List Archive mirror
 help / color / mirror / Atom feed
From: Sergey Organov <sorganov@gmail.com>
To: Felipe Contreras <felipe.contreras@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH v1 0/7] diff: fix -s and --no-patch
Date: Fri, 12 May 2023 12:32:18 +0300	[thread overview]
Message-ID: <87r0rlj3od.fsf@osv.gnss.ru> (raw)
In-Reply-To: <645df6e614f00_215cec29462@chronos.notmuch> (Felipe Contreras's message of "Fri, 12 May 2023 02:20:54 -0600")

Felipe Contreras <felipe.contreras@gmail.com> writes:

> Felipe Contreras wrote:
>> This fixes an issue Sergey Organov reported.
>
> Sergey, as you can see this series fixes the issue you reported.
>
> First, I think these should remain working the same, simply for convenience:
>
>  * git diff         # default output
>  * git diff --patch # patch output
>  * git diff --raw   # raw output
>  * git diff --stat  # stat output
>
> I don't think there's a way I can be convinced otherwise.

Fine with me.

>
> But there's many changes:
>
>  1. git diff -s --raw                 # before: nil, after: raw
>  2. git diff --no-patch --raw         # before: nil, after: raw
>  3. git diff --patch --no-patch --raw # before: nil, after: raw
>  4. git diff --raw --patch --no-patch # before: nil, after: raw

Fine as well.

>
> I don't think there's any way you can say my 174 changes make the code work
> "exactly the same".

I said that in the context where we discussed entirely separate issue
"handling of defaults by Git commands". Irrelevant to these series as
they don't touch this aspect as visible from outside, even though you
do change the implementation for better.

>
> And this is better than Junio's solution, because #4 outputs a raw format,
> while in Junio's solution it doesn't output anything.

Yes, and that's where I agreed from the very beginning.

> Even if you don't agree with everything, this solution is better than the
> status quo, and it's better than Junio's solution as it fixes --no-patch
> immediately.

Yep, it fixes "--no-patch" semantics indeed, and as I already said, I do
vote in favor of this change, for what it's worth.

Thanks,
-- Sergey Organov

      reply	other threads:[~2023-05-12  9:32 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-12  8:03 [PATCH v1 0/7] diff: fix -s and --no-patch Felipe Contreras
2023-05-12  8:03 ` [PATCH v1 1/7] line-log: set patch format explicitly by default Felipe Contreras
2023-05-12  8:03 ` [PATCH v1 2/7] diff: introduce DIFF_FORMAT_DEFAULT Felipe Contreras
2023-05-12  8:03 ` [PATCH v1 3/7] diff: make DIFF_FORMAT_NO_OUTPUT 0 Felipe Contreras
2023-05-12  8:03 ` [PATCH v1 4/7] test: add various tests for diff formats with -s Felipe Contreras
2023-05-12  8:03 ` [PATCH v1 5/7] diff: split --no-patch from -s Felipe Contreras
2023-05-12  8:03 ` [PATCH v1 6/7] diff: add --silent as alias of -s Felipe Contreras
2023-05-12  8:03 ` [PATCH v1 7/7] diff: remove DIFF_FORMAT_NO_OUTPUT Felipe Contreras
2023-05-12  8:20 ` [PATCH v1 0/7] diff: fix -s and --no-patch Felipe Contreras
2023-05-12  9:32   ` Sergey Organov [this message]

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=87r0rlj3od.fsf@osv.gnss.ru \
    --to=sorganov@gmail.com \
    --cc=felipe.contreras@gmail.com \
    --cc=git@vger.kernel.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).