git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Jonathan Nieder <jrnieder@gmail.com>
Cc: "Matthieu Moy" <Matthieu.Moy@grenoble-inp.fr>,
	"Junio C Hamano" <gitster@pobox.com>,
	git@vger.kernel.org, "Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
Subject: Re: [PATCH 0/4] make pathless 'add [-u|-A]' warning less noisy
Date: Tue, 19 Mar 2013 00:25:14 -0400	[thread overview]
Message-ID: <20130319042514.GA4807@sigill.intra.peff.net> (raw)
In-Reply-To: <20130319034415.GI5062@elie.Belkin>

On Mon, Mar 18, 2013 at 08:44:15PM -0700, Jonathan Nieder wrote:

> >                                                          The
> > config option added by this patch gives them such an option.
> 
> I suspect the need for this config option is a sign that the warning
> is too eager.  After all, the whole idea of the change being safe is
> that it shouldn't make a difference the way people usually use git,
> no?
> 
> In other words, how about the following patches?  With them applied,
> hopefully no one would mind even if the warning becomes a fatal error.

Clever. I think it would help in my case. I sometimes follow the
workflow you describe in patch 3 (i.e., just working in a subdir), and
sometimes do something more like:

  $ vi foo.c
  $ cd t
  $ vi tXXXX-foo.sh
  $ ./tXXXX-foo.sh
  $ git add -u

With your patches, we would continue to warn about the second case, but
I think that is a good thing; git is not doing what I want. But by
reducing the false positives from the first case, I would start to
actually pay attention to the warning more.

> Jonathan Nieder (4):
>   add: make pathless 'add [-u|-A]' warning a file-global function
>   add: make warn_pathless_add() a no-op after first call
>   add -u: only show pathless 'add -u' warning when changes exist outside cwd
>   add -A: only show pathless 'add -A' warning when changes exist outside cwd

I don't see anything obviously wrong with the patches themselves. I
wonder if we would want to change the warning to be more explicit that
yes, there really were files that were impacted by this. And possibly
even list them.

I suspect I would not even mind that becoming the final behavior.  I.e.,
going to:

  $ cd subdir && git add -u
  warning: Using 'git add -u' without a pathspec operates only on the
  current subdirectory. Updates from the following files were NOT
  staged:

    file1
    file2
    other-subdir/file3

now, and then eventually converting the warning into a fatal error (and
demanding that the user use ":/" or "." as appropriate).

But in the long run, I guess defaulting to ":/" will be more convenient,
so there is no point in complaining about the ambiguity forever. And in
that case, since the warning is just a placeholder, I don't know that
it's worth much effort to make it fancier.

-Peff

  parent reply	other threads:[~2013-03-19  4:25 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-03-08 23:54 [PATCH 0/2] "git add -u/-A" from future Junio C Hamano
2013-03-08 23:54 ` [PATCH 1/2] require pathspec for "git add -u/-A" Junio C Hamano
2013-03-10 15:49   ` Matthieu Moy
2013-03-11  7:04     ` Junio C Hamano
2013-03-11  8:00       ` Matthieu Moy
2013-03-11  8:01         ` [PATCH 1/2] add: update pathless 'add [-u|-A]' warning to reflect change of plan Matthieu Moy
2013-03-11  8:01           ` [PATCH 2/2] add: add a newline at the end of pathless 'add [-u|-A]' warning Matthieu Moy
2013-03-11 16:06             ` Junio C Hamano
2013-04-02 14:43               ` Matthieu Moy
2013-04-02 16:31                 ` Junio C Hamano
2013-04-02 16:57                   ` Matthieu Moy
2013-03-12 11:28     ` [PATCH 1/2] require pathspec for "git add -u/-A" Jeff King
2013-03-12 13:58       ` Matthieu Moy
2013-03-13  4:08         ` Jeff King
2013-03-13  4:10           ` [PATCH 1/2] t2200: check that "add -u" limits itself to subdirectory Jeff King
2013-03-13  8:52             ` Matthieu Moy
2013-03-13 17:44             ` Junio C Hamano
2013-03-14  6:44               ` Jeff King
2013-03-13  4:10           ` [PATCH 2/2] add: respect add.updateroot config option Jeff King
2013-03-13  9:07             ` Matthieu Moy
2013-03-13  9:27               ` Jeff King
2013-03-13 15:51                 ` Junio C Hamano
2013-03-14 12:39                   ` Matthieu Moy
2013-03-19  3:44             ` [PATCH 0/4] make pathless 'add [-u|-A]' warning less noisy Jonathan Nieder
2013-03-19  3:45               ` [PATCH 1/4] add: make pathless 'add [-u|-A]' warning a file-global function Jonathan Nieder
2013-03-19  3:46               ` [PATCH 2/4] add: make warn_pathless_add() a no-op after first call Jonathan Nieder
2013-03-19  3:48               ` [PATCH 3/4] add -u: only show pathless 'add -u' warning when changes exist outside cwd Jonathan Nieder
2013-03-19  4:25                 ` Junio C Hamano
2013-03-19  5:28                   ` Jonathan Nieder
2013-03-19 14:57                     ` Junio C Hamano
2013-03-19  5:34                   ` Jonathan Nieder
2013-03-19  5:37                   ` Duy Nguyen
2013-03-19  5:44                     ` Jonathan Nieder
2013-03-19  6:21                 ` Matthieu Moy
2013-03-19 15:06                   ` Junio C Hamano
2013-03-19 19:06                     ` Jonathan Nieder
2013-03-19 19:47                       ` Junio C Hamano
2013-03-19 20:34                         ` Jonathan Nieder
2013-03-19  3:49               ` [PATCH 4/4] add -A: only show pathless 'add -A' " Jonathan Nieder
2013-03-19  4:25               ` Jeff King [this message]
2013-03-08 23:54 ` [PATCH 2/2] git add: -u/-A now affects the entire working tree Junio C Hamano
2013-03-19 22:44 ` [PATCH v2 0/6] make pathless 'add [-u|-A]' warning less noisy Jonathan Nieder
2013-03-19 22:44   ` [PATCH 1/6] t2200: check that "add -u" limits itself to subdirectory Jonathan Nieder
2013-03-19 22:45   ` [PATCH 2/6] add: make pathless 'add [-u|-A]' warning a file-global function Jonathan Nieder
2013-03-19 22:45   ` [PATCH 3/6] add: make warn_pathless_add() a no-op after first call Jonathan Nieder
2013-03-19 22:50   ` [PATCH 4/6] add -u: only show pathless 'add -u' warning when changes exist outside cwd Jonathan Nieder
2013-03-20  5:06     ` Jeff King
2013-03-20 15:10     ` Junio C Hamano
2013-03-19 22:51   ` [PATCH 5/6] add -A: only show pathless 'add -A' " Jonathan Nieder
2013-03-20 15:30     ` Junio C Hamano
2013-03-19 22:53   ` [PATCH 6/6] git add: -u/-A now affects the entire working tree Jonathan Nieder

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=20130319042514.GA4807@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=Matthieu.Moy@grenoble-inp.fr \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jrnieder@gmail.com \
    --cc=pclouds@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 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 NNTP newsgroup(s).