All of lore.kernel.org
 help / color / mirror / Atom feed
From: Anthony Foiani <anthony.foiani@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH] Improve errors from 'git diff --no-index'.
Date: Mon, 23 May 2011 15:24:50 -0600	[thread overview]
Message-ID: <BANLkTinmtJXo4jcnB1TAFPPtcGqun5rfoA@mail.gmail.com> (raw)
In-Reply-To: <7vpqn9usqt.fsf@alter.siamese.dyndns.org>

Junio --

This fundamentally all started as me confusing myself, and the patch
was the result of me scratching that itch.  As such, I'm perfectly
willing to drop it, but I did want to address a few last things:

On Mon, May 23, 2011 at 1:22 PM, Junio C Hamano <gitster@pobox.com> wrote:

>> $ # with my patch:
>> $ ../git/git-diff /tmp/{foo,bar}
>> warning: neither '/tmp/foo' nor '/tmp/bar' are tracked, forcing --no-index
>
> I actually consider this a regression. We are giving an output that the
> user wanted to see, and I do not see a reason why we need to warn.

No, we're not: the user did *not* ask for a no-index diff, but we're
giving them one anyway. That warrants a warn (or other message; if
there's a less-severe version of "warn", then that's fine) to tell
them that the software is trying to "do what you mean".

Please also keep in mind that you live and breathe git, and have done
so for years (and I'm very thankful for that!).  But not everyone has
the git mindset all the time; adding an extra message or two doesn't
seem like too much pain to help novices and dilettantes.

> But that consistency goes totally against what the users would expect.
> This inconsistency is not a fault of either the definition of "git diff"
> nor the user's expectation. They are fundamentally different and the root
> cause of it is that we support --no-index diff between randomly chosen two
> files.

Maybe we *should* drop the feature: I would have been *happier* had
the error message from my initial attempt been simply: "not in a git
repo, can't use git diff". (Since that was, fundamentally, the mistake
I made.)

Maybe install "diff-git" or even "gdiff" (although maybe that's used
by gnu diff on non-gnu platforms, dunno).

But I maintain that the current combination of options, assumptions,
and error output are insufficient, and I still feel that my patch(es)
improve that matter.  *shrug*  Your call, of course.

Best regards,
Tony

      parent reply	other threads:[~2011-05-23 21:24 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-05-22 19:17 [PATCH] Improve errors from 'git diff --no-index' Anthony Foiani
     [not found] ` <7vlixyw4cx.fsf@alter.siamese.dyndns.org>
2011-05-23  2:35   ` Anthony Foiani
2011-05-23  3:18     ` Junio C Hamano
2011-05-23  4:05       ` Anthony Foiani
2011-05-23 19:22         ` Junio C Hamano
2011-05-23 20:50           ` Jeff King
2011-05-23 21:24           ` Anthony Foiani [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=BANLkTinmtJXo4jcnB1TAFPPtcGqun5rfoA@mail.gmail.com \
    --to=anthony.foiani@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.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.