Git development
 help / color / mirror / Atom feed
* Re: "git add -p" versus "git add -i", followed by "p"
From: SZEDER Gábor @ 2018-12-02 16:56 UTC (permalink / raw)
  To: Robert P. J. Day; +Cc: Git Mailing list
In-Reply-To: <alpine.LFD.2.21.1812021124350.5509@localhost.localdomain>

On Sun, Dec 02, 2018 at 11:30:19AM -0500, Robert P. J. Day wrote:
> 
>   testing adding by patch for the very first time (i've just never
> needed this), and reading the "progit" book and reading the man page,
> and the impression i'm getting is that running "git add -p" (going
> straight to patch mode) is supposed to be equivalent to running "git
> add -i", then typing "p" to switch to patch mode.
> 
>   that is most emphatically not what i'm seeing. if i run "git add
> -p", then i get to what i expect -- the patch subsystem:
> 
>   $ git add -p
>   diff --git a/README.asc b/README.asc
>   index fa40bad..840e85b 100644
>   --- a/README.asc
>   +++ b/README.asc
>   @@ -1,3 +1,9 @@
>   +change 1
>   +
>   +
>   +
>   +
>   +
>    = Pro Git, Second Edition
> 
>    Welcome to the second edition of the Pro Git book.
>   Stage this hunk [y,n,q,a,d,j,J,g,/,e,?]?
> 
> but if i start with "git add -i", there seems to be no way to get to
> patch mode -- certainly "p" doesn't do it. am i stupidly missing
> something trivial? is the explanation misleading or inncomplete?

Worksforme™:

  $ echo "New content" >>README.md 
  $ echo "New content" >>t/README
  $ echo "New content" >>contrib//README
  $ git add -i
             staged     unstaged path
    1:    unchanged        +1/-0 README.md
    2:    unchanged        +1/-0 contrib/README
    3:    unchanged        +1/-0 t/README
  
  *** Commands ***
    1: status       2: update       3: revert       4: add untracked
    5: patch        6: diff         7: quit         8: help
  What now> p
             staged     unstaged path
    1:    unchanged        +1/-0 README.md
    2:    unchanged        +1/-0 contrib/README
    3:    unchanged        +1/-0 t/README
  Patch update>> 1
             staged     unstaged path
  * 1:    unchanged        +1/-0 README.md
    2:    unchanged        +1/-0 contrib/README
    3:    unchanged        +1/-0 t/README
  Patch update>> 2
             staged     unstaged path
  * 1:    unchanged        +1/-0 README.md
  * 2:    unchanged        +1/-0 contrib/README
    3:    unchanged        +1/-0 t/README
  Patch update>> 

Here I hit enter.  Did you?

  diff --git a/README.md b/README.md
  index f920a42fad..63dee5cfc3 100644
  --- a/README.md
  +++ b/README.md
  @@ -62,3 +62,4 @@ and the name as (depending on your mood):
   [Documentation/giteveryday.txt]: Documentation/giteveryday.txt
   [Documentation/gitcvs-migration.txt]:
  Documentation/gitcvs-migration.txt
   [Documentation/SubmittingPatches]: Documentation/SubmittingPatches
  +New content
  Stage this hunk [y,n,q,a,d,e,?]? y
  
  diff --git a/contrib/README b/contrib/README
  index 05f291c1f1..2b152dfcff 100644
  --- a/contrib/README
  +++ b/contrib/README
  @@ -41,3 +41,4 @@ submit a patch to create a subdirectory of contrib/
  and put your
   stuff there.
   
   -jc
  +New content
  Stage this hunk [y,n,q,a,d,e,?]? n
  
  *** Commands ***
    1: status       2: update       3: revert       4: add untracked
    5: patch        6: diff         7: quit         8: help
  What now> q
  Bye.
  $ git diff --cached 
  diff --git a/README.md b/README.md
  index f920a42fad..63dee5cfc3 100644
  --- a/README.md
  +++ b/README.md
  @@ -62,3 +62,4 @@ and the name as (depending on your mood):
   [Documentation/giteveryday.txt]: Documentation/giteveryday.txt
   [Documentation/gitcvs-migration.txt]: Documentation/gitcvs-migration.txt
   [Documentation/SubmittingPatches]: Documentation/SubmittingPatches
  +New content
  $


Arguably the documentation could make it clear that the user can
choose multiple files at once, e.g.:

diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
index c9623854bf..061f9cbb0d 100644
--- a/Documentation/git-add.txt
+++ b/Documentation/git-add.txt
@@ -317,9 +317,9 @@ add untracked::
 
 patch::
 
-  This lets you choose one path out of a 'status' like selection.
-  After choosing the path, it presents the diff between the index
-  and the working tree file and asks you if you want to stage
+  This lets you choose one or more paths out of a 'status' like selection.
+  After choosing the path(s), it presents the diff between the index
+  and the working tree file(s) and asks you if you want to stage
   the change of each hunk.  You can select one of the following
   options and type return:
 
And perhaps we could have a dedicated menu entry for "I'm done with
selecting paths"?  Dunno; I'm a 'git add -p' user myself.


^ permalink raw reply related

* Re: "git add -p" versus "git add -i", followed by "p"
From: Kevin Daudt @ 2018-12-02 16:52 UTC (permalink / raw)
  To: Robert P. J. Day; +Cc: Git Mailing list
In-Reply-To: <alpine.LFD.2.21.1812021124350.5509@localhost.localdomain>

On Sun, Dec 02, 2018 at 11:30:19AM -0500, Robert P. J. Day wrote:
> 
>   testing adding by patch for the very first time (i've just never
> needed this), and reading the "progit" book and reading the man page,
> and the impression i'm getting is that running "git add -p" (going
> straight to patch mode) is supposed to be equivalent to running "git
> add -i", then typing "p" to switch to patch mode.
> 
>   that is most emphatically not what i'm seeing. if i run "git add
> -p", then i get to what i expect -- the patch subsystem:
> 
>   $ git add -p
>   diff --git a/README.asc b/README.asc
>   index fa40bad..840e85b 100644
>   --- a/README.asc
>   +++ b/README.asc
>   @@ -1,3 +1,9 @@
>   +change 1
>   +
>   +
>   +
>   +
>   +
>    = Pro Git, Second Edition
> 
>    Welcome to the second edition of the Pro Git book.
>   Stage this hunk [y,n,q,a,d,j,J,g,/,e,?]?
> 
> but if i start with "git add -i", there seems to be no way to get to
> patch mode -- certainly "p" doesn't do it. am i stupidly missing
> something trivial? is the explanation misleading or inncomplete?
> 
> rday
> 
> -- 
> 

After selecting 'p', what do you get?

You should see a list of modified files. You can select the files you
want to stage by the listed numbers. After you selected those files, you
press enter, and then you will get the options you'll also see with git
add -p.

Kevin

^ permalink raw reply

* Re: "git add -p" versus "git add -i", followed by "p"
From: Robert P. J. Day @ 2018-12-02 16:59 UTC (permalink / raw)
  To: Kevin Daudt; +Cc: Git Mailing list
In-Reply-To: <20181202165242.GA4823@alpha>

On Sun, 2 Dec 2018, Kevin Daudt wrote:

> On Sun, Dec 02, 2018 at 11:30:19AM -0500, Robert P. J. Day wrote:
> >
> >   testing adding by patch for the very first time (i've just never
> > needed this), and reading the "progit" book and reading the man page,
> > and the impression i'm getting is that running "git add -p" (going
> > straight to patch mode) is supposed to be equivalent to running "git
> > add -i", then typing "p" to switch to patch mode.
> >
> >   that is most emphatically not what i'm seeing. if i run "git add
> > -p", then i get to what i expect -- the patch subsystem:
> >
> >   $ git add -p
> >   diff --git a/README.asc b/README.asc
> >   index fa40bad..840e85b 100644
> >   --- a/README.asc
> >   +++ b/README.asc
> >   @@ -1,3 +1,9 @@
> >   +change 1
> >   +
> >   +
> >   +
> >   +
> >   +
> >    = Pro Git, Second Edition
> >
> >    Welcome to the second edition of the Pro Git book.
> >   Stage this hunk [y,n,q,a,d,j,J,g,/,e,?]?
> >
> > but if i start with "git add -i", there seems to be no way to get
> > to patch mode -- certainly "p" doesn't do it. am i stupidly
> > missing something trivial? is the explanation misleading or
> > inncomplete?
>
> After selecting 'p', what do you get?
>
> You should see a list of modified files. You can select the files
> you want to stage by the listed numbers. After you selected those
> files, you press enter, and then you will get the options you'll
> also see with git add -p.

$ git add -i
           staged     unstaged path
  1:    unchanged       +12/-0 README.asc

*** Commands ***
  1: [s]tatus	  2: [u]pdate	  3: [r]evert	  4: [a]dd untracked
  5: [p]atch	  6: [d]iff	  7: [q]uit	  8: [h]elp
What now> p
           staged     unstaged path
  1:    unchanged       +12/-0 [R]EADME.asc
Patch update>> 1
           staged     unstaged path
* 1:    unchanged       +12/-0 [R]EADME.asc
Patch update>> 1
           staged     unstaged path
* 1:    unchanged       +12/-0 [R]EADME.asc
Patch update>>

  and ... then what?

rday

-- 

========================================================================
Robert P. J. Day                                 Ottawa, Ontario, CANADA
                  http://crashcourse.ca/dokuwiki

Twitter:                                       http://twitter.com/rpjday
LinkedIn:                               http://ca.linkedin.com/in/rpjday
========================================================================

^ permalink raw reply

* Re: "git add -p" versus "git add -i", followed by "p"
From: Robert P. J. Day @ 2018-12-02 17:01 UTC (permalink / raw)
  To: SZEDER Gábor; +Cc: Git Mailing list
In-Reply-To: <20181202165617.GG30222@szeder.dev>

[-- Attachment #1: Type: text/plain, Size: 2887 bytes --]

On Sun, 2 Dec 2018, SZEDER Gábor wrote:

> On Sun, Dec 02, 2018 at 11:30:19AM -0500, Robert P. J. Day wrote:
> >
> >   testing adding by patch for the very first time (i've just never
> > needed this), and reading the "progit" book and reading the man page,
> > and the impression i'm getting is that running "git add -p" (going
> > straight to patch mode) is supposed to be equivalent to running "git
> > add -i", then typing "p" to switch to patch mode.
> >
> >   that is most emphatically not what i'm seeing. if i run "git add
> > -p", then i get to what i expect -- the patch subsystem:
> >
> >   $ git add -p
> >   diff --git a/README.asc b/README.asc
> >   index fa40bad..840e85b 100644
> >   --- a/README.asc
> >   +++ b/README.asc
> >   @@ -1,3 +1,9 @@
> >   +change 1
> >   +
> >   +
> >   +
> >   +
> >   +
> >    = Pro Git, Second Edition
> >
> >    Welcome to the second edition of the Pro Git book.
> >   Stage this hunk [y,n,q,a,d,j,J,g,/,e,?]?
> >
> > but if i start with "git add -i", there seems to be no way to get to
> > patch mode -- certainly "p" doesn't do it. am i stupidly missing
> > something trivial? is the explanation misleading or inncomplete?
>
> Worksforme™:
>
>   $ echo "New content" >>README.md
>   $ echo "New content" >>t/README
>   $ echo "New content" >>contrib//README
>   $ git add -i
>              staged     unstaged path
>     1:    unchanged        +1/-0 README.md
>     2:    unchanged        +1/-0 contrib/README
>     3:    unchanged        +1/-0 t/README
>
>   *** Commands ***
>     1: status       2: update       3: revert       4: add untracked
>     5: patch        6: diff         7: quit         8: help
>   What now> p
>              staged     unstaged path
>     1:    unchanged        +1/-0 README.md
>     2:    unchanged        +1/-0 contrib/README
>     3:    unchanged        +1/-0 t/README
>   Patch update>> 1
>              staged     unstaged path
>   * 1:    unchanged        +1/-0 README.md
>     2:    unchanged        +1/-0 contrib/README
>     3:    unchanged        +1/-0 t/README
>   Patch update>> 2
>              staged     unstaged path
>   * 1:    unchanged        +1/-0 README.md
>   * 2:    unchanged        +1/-0 contrib/README
>     3:    unchanged        +1/-0 t/README
>   Patch update>>
>
> Here I hit enter.  Did you?

  ah, so after selecting the files to selectively stage, one enters a
simple CR. got it. not sure that's obvious from the docs but i'll
verify that.

rday

-- 

========================================================================
Robert P. J. Day                                 Ottawa, Ontario, CANADA
                  http://crashcourse.ca/dokuwiki

Twitter:                                       http://twitter.com/rpjday
LinkedIn:                               http://ca.linkedin.com/in/rpjday
========================================================================

^ permalink raw reply

* Re: "git add -p" versus "git add -i", followed by "p"
From: Robert P. J. Day @ 2018-12-02 17:03 UTC (permalink / raw)
  To: SZEDER Gábor; +Cc: Git Mailing list
In-Reply-To: <20181202165617.GG30222@szeder.dev>

[-- Attachment #1: Type: text/plain, Size: 2924 bytes --]

On Sun, 2 Dec 2018, SZEDER Gábor wrote:

> On Sun, Dec 02, 2018 at 11:30:19AM -0500, Robert P. J. Day wrote:
> >
> >   testing adding by patch for the very first time (i've just never
> > needed this), and reading the "progit" book and reading the man page,
> > and the impression i'm getting is that running "git add -p" (going
> > straight to patch mode) is supposed to be equivalent to running "git
> > add -i", then typing "p" to switch to patch mode.
> >
> >   that is most emphatically not what i'm seeing. if i run "git add
> > -p", then i get to what i expect -- the patch subsystem:
> >
> >   $ git add -p
> >   diff --git a/README.asc b/README.asc
> >   index fa40bad..840e85b 100644
> >   --- a/README.asc
> >   +++ b/README.asc
> >   @@ -1,3 +1,9 @@
> >   +change 1
> >   +
> >   +
> >   +
> >   +
> >   +
> >    = Pro Git, Second Edition
> >
> >    Welcome to the second edition of the Pro Git book.
> >   Stage this hunk [y,n,q,a,d,j,J,g,/,e,?]?
> >
> > but if i start with "git add -i", there seems to be no way to get to
> > patch mode -- certainly "p" doesn't do it. am i stupidly missing
> > something trivial? is the explanation misleading or inncomplete?
>
> Worksforme™:
>
>   $ echo "New content" >>README.md
>   $ echo "New content" >>t/README
>   $ echo "New content" >>contrib//README
>   $ git add -i
>              staged     unstaged path
>     1:    unchanged        +1/-0 README.md
>     2:    unchanged        +1/-0 contrib/README
>     3:    unchanged        +1/-0 t/README
>
>   *** Commands ***
>     1: status       2: update       3: revert       4: add untracked
>     5: patch        6: diff         7: quit         8: help
>   What now> p
>              staged     unstaged path
>     1:    unchanged        +1/-0 README.md
>     2:    unchanged        +1/-0 contrib/README
>     3:    unchanged        +1/-0 t/README
>   Patch update>> 1
>              staged     unstaged path
>   * 1:    unchanged        +1/-0 README.md
>     2:    unchanged        +1/-0 contrib/README
>     3:    unchanged        +1/-0 t/README
>   Patch update>> 2
>              staged     unstaged path
>   * 1:    unchanged        +1/-0 README.md
>   * 2:    unchanged        +1/-0 contrib/README
>     3:    unchanged        +1/-0 t/README
>   Patch update>>
>
> Here I hit enter.  Did you?

  perhaps i'm just not seeing it, but from "man git-add", it doesn't
seem obvious that you would first select the files to work with, then
hit a simple CR to get into actual patch mode.

rday

-- 

========================================================================
Robert P. J. Day                                 Ottawa, Ontario, CANADA
                  http://crashcourse.ca/dokuwiki

Twitter:                                       http://twitter.com/rpjday
LinkedIn:                               http://ca.linkedin.com/in/rpjday
========================================================================

^ permalink raw reply

* Re: "git add -p" versus "git add -i", followed by "p"
From: Duy Nguyen @ 2018-12-02 17:09 UTC (permalink / raw)
  To: Robert P. J. Day; +Cc: SZEDER Gábor, Git Mailing List
In-Reply-To: <alpine.LFD.2.21.1812021201550.6459@localhost.localdomain>

On Sun, Dec 2, 2018 at 6:05 PM Robert P. J. Day <rpjday@crashcourse.ca> wrote:
> >   Patch update>> 2
> >              staged     unstaged path
> >   * 1:    unchanged        +1/-0 README.md
> >   * 2:    unchanged        +1/-0 contrib/README
> >     3:    unchanged        +1/-0 t/README
> >   Patch update>>
> >
> > Here I hit enter.  Did you?
>
>   perhaps i'm just not seeing it, but from "man git-add", it doesn't
> seem obvious that you would first select the files to work with, then
> hit a simple CR to get into actual patch mode.

I think it's the same procedure as the "update" step, which describes
this in more detail. I agree that the "patch" section does not make
this obvious.
-- 
Duy

^ permalink raw reply

* Re: "git add -p" versus "git add -i", followed by "p"
From: Robert P. J. Day @ 2018-12-02 17:30 UTC (permalink / raw)
  To: Duy Nguyen; +Cc: SZEDER Gábor, Git Mailing List
In-Reply-To: <CACsJy8AkMfZ02b9p2sQi2p=Bw4MDckLYy_cBFVeN2_UY-Z3kCg@mail.gmail.com>

On Sun, 2 Dec 2018, Duy Nguyen wrote:

> On Sun, Dec 2, 2018 at 6:05 PM Robert P. J. Day <rpjday@crashcourse.ca> wrote:
> > >   Patch update>> 2
> > >              staged     unstaged path
> > >   * 1:    unchanged        +1/-0 README.md
> > >   * 2:    unchanged        +1/-0 contrib/README
> > >     3:    unchanged        +1/-0 t/README
> > >   Patch update>>
> > >
> > > Here I hit enter.  Did you?
> >
> >   perhaps i'm just not seeing it, but from "man git-add", it
> > doesn't seem obvious that you would first select the files to work
> > with, then hit a simple CR to get into actual patch mode.
>
> I think it's the same procedure as the "update" step, which
> describes this in more detail. I agree that the "patch" section does
> not make this obvious.

  thanks, i was hoping i wasn't being a complete idiot. given time, i
may submit a patch to fix the man page unless someone else gets to it
first.

rday

-- 

========================================================================
Robert P. J. Day                                 Ottawa, Ontario, CANADA
                  http://crashcourse.ca/dokuwiki

Twitter:                                       http://twitter.com/rpjday
LinkedIn:                               http://ca.linkedin.com/in/rpjday
========================================================================

^ permalink raw reply

* RE: [RFC] git clean --local
From: Randall S. Becker @ 2018-12-02 17:37 UTC (permalink / raw)
  To: 'Ævar Arnfjörð Bjarmason',
	'Cameron Boehmer'
  Cc: git
In-Reply-To: <87woosukkm.fsf@evledraar.gmail.com>

On December 2, 2018 8:26, Ævar Arnfjörð Bjarmason wrote:
> 
> On Sat, Dec 01 2018, Cameron Boehmer wrote:
> 
> > 1) add a new flag
> > -l, --local
> >     Do not consult git config --global core.excludesFile in
> > determining what files git ignores. This is useful in conjunction with
> > -x/-X to preserve user files while removing build artifacts.
> 
> Or perhaps a general flag to ignore configuration would be useful for such
> cases, see https://public-
> inbox.org/git/87zhtqvm66.fsf@evledraar.gmail.com/

Would something like git clean --exclude=file-pattern work as a compromise notion? Files matching the pattern would not be cleaned regardless of .gitignore or their potential preciousness status long-term. Multiple repetitions of the --exclude option might be supportable. I could see that being somewhat useful in scripting.

Cheers,
Randall



^ permalink raw reply

* Re: [PATCH/RFC v3 00/14] Introduce new commands switch-branch and restore-files
From: Thomas Gummerer @ 2018-12-02 18:58 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Nguyễn Thái Ngọc Duy, avarab, git, sbeller,
	sxenos
In-Reply-To: <xmqqefb3mhrs.fsf@gitster-ct.c.googlers.com>

On 11/30, Junio C Hamano wrote:
> 
> I am unsure about the wisdom of calling it "--index", though.
> 
> The "--index" option is "the command can work only on the index, or
> only on the working tree files, or on both the index and the working
> tree files, and this option tells it to work in the 'both the index
> and the working tree files' mode", but when "restore-files" touches
> paths, it always modifies both the index and the working tree, so
> the "--index" option does not capture the differences well in this
> context [*1*].  As I saw this was described as "not using the usual
> 'overlay' semantics [*2*]", perhaps --overlay/--no-overlay option
> that defaults to --no-overlay is easier to explain.

Agreed, I think --{no-,}overlay is a much better name for the option,
I'll use that for my patch series (I hope to send that soon after 2.20
is released).

I must admit that I was not aware that the mode is called overlay
mode, before you explained it to me, so I wouldn't expect most users
to know either.  But as it's easy to explain that probably doesn't
matter much.

>     side note 1.  I think the original mention of "--index" came in
>     the context of contrasting "git reset" with "git checkout".
>     "git reset (--hard|--mixed) -- <pathspec>" (that does not move
>     HEAD), which does not but perhaps should exist, is very much
>     like "git checkout -- <pathspec>", and if "reset" were written
>     after the "--index/--cached" convention was established, "reset
>     --hard" would have called "reset --index" while "reset --mixed"
>     would have been "reset --cached" (i.e. only work on the index
>     and not on the working tree).  And "reset --index <directory>"
>     would have worked by removing paths in <directory> that are not
>     in the HEAD and updating paths in <directory> that are in the
>     HEAD, i.e. identical to the non overlay behaviour proposed for
>     the "git checkout" command.  So calling the non overlay mode
>     "--index" makes sense in the context of discussing "git reset",
>     and not in the context of "git checkout".
> 
>     side note 2.  "git checkout <tree-ish> <pathspec>" grabs entries
>     from the <tree-ish> that patch <pathspec> and adds them to the
>     index and checks them out to the working tree.  If the original
>     index has entries that match <pathspec> but do not appear in
>     <tree-ish>, they are left in the result.  That is "overlaying
>     what was taken from the <tree-ish> on top of what is in the
>     index".
> 
> Having said all that, I will not be looking at the series until 2.20
> final.  And I hope more people do the same to concentrate on helping
> us prevent the last minute glitch slipping in the final release.
> 
> Thanks.

^ permalink raw reply

* Re: BUG: CR marker ^M doesn't show up in '-' lines of diffs when the ending of the removed line is CR+LF
From: Frank Schäfer @ 2018-12-02 19:31 UTC (permalink / raw)
  To: Junio C Hamano, Johannes Sixt; +Cc: brian m. carlson, git
In-Reply-To: <xmqqk1kwr5tp.fsf@gitster-ct.c.googlers.com>

Hi Junio,

Am 29.11.18 um 03:11 schrieb Junio C Hamano:
[...]
> This was misspoken a bit.  Let's revise it to
>
>  	When producing a colored output (not limited to whitespace
>  	error coloring of diff output) for contents that are not
>  	marked as eol=crlf (and other historical means), insert
>  	<RESET> before a CR that comes immediately before a LF.
You mean
     ...
     <RESET> *after* a CR that comes immediately before a LF."


OK, AFAICS this produces the desired output in all cases if eol=lf.

Now what about the case eol=crlf ?
Keeping the current behavior of '-' lines is correct.
But shouldn't ^M now be suppressed in '+' lines for a consistent behavior ?

With other words:
"If CR comes immediately before a LF, do the following with *all* lines:
<RESET> after the CR if eol=lf but do not <RESET> after the CR if eol=crlf."

Regards,
Frank

^ permalink raw reply

* Re: [RFC] git clean --local
From: Junio C Hamano @ 2018-12-02 19:37 UTC (permalink / raw)
  To: Randall S. Becker
  Cc: 'Ævar Arnfjörð Bjarmason',
	'Cameron Boehmer', git
In-Reply-To: <004101d48a65$afb0da40$0f128ec0$@nexbridge.com>

"Randall S. Becker" <rsbecker@nexbridge.com> writes:


> Would something like git clean --exclude=file-pattern work as a
> compromise notion? Files matching the pattern would not be cleaned
> regardless of .gitignore or their potential preciousness status
> long-term. Multiple repetitions of the --exclude option might be
> supportable. I could see that being somewhat useful in scripting.

I think "git clean" already takes "-e", but I am not sure if it is
meant to do what you wrote above.

If "git clean" takes a pathspec, perhaps you can give a negative
pathspec to exclude whatever you do not want to get cleaned,
something like

	git clean '*.o' ':!precious.o'

to say "presious.o is ignored (hence normally expendable), but I do
not want to clean it with this invocation of 'git clean'"?

^ permalink raw reply

* Re: [L10N] Kickoff for Git 2.20.0 round 3
From: Junio C Hamano @ 2018-12-02 19:39 UTC (permalink / raw)
  To: Jiang Xin
  Cc: Alexander Shopov, Jordi Mas, Ralf Thielow, Christopher Díaz,
	Jean-Noël Avila, Marco Paolone, Gwan-gyeong Mun,
	Vasco Almeida, Dimitriy Ryazantcev, Peter Krefting,
	Trần Ngọc Quân, Git List
In-Reply-To: <CANYiYbG6X4Z02bQNJzWr73DbyR58Wd6yq+z0KngTNfhkHZ6txw@mail.gmail.com>

Jiang Xin <worldhello.net@gmail.com> writes:

> Git v2.20.0-rc2 has been released, and there are 5 new messages need to
> be translated. So let's start new round of l10n for Git 2.20.0.

A huge thanks, as always, to the translation team.  Jiang, sorry to
see that -rc2 slipped just after you sent out the round 2 message
and you needed to start round 3 just after it.


^ permalink raw reply

* Re: [PATCH/RFC v3 00/14] Introduce new commands switch-branch and restore-files
From: Junio C Hamano @ 2018-12-02 19:46 UTC (permalink / raw)
  To: Thomas Gummerer
  Cc: Nguyễn Thái Ngọc Duy, avarab, git, sbeller,
	sxenos
In-Reply-To: <20181202185838.GN4883@hank.intra.tgummerer.com>

Thomas Gummerer <t.gummerer@gmail.com> writes:

> Agreed, I think --{no-,}overlay is a much better name for the option,
> I'll use that for my patch series (I hope to send that soon after 2.20
> is released).

OK.

> I must admit that I was not aware that the mode is called overlay
> mode, before you explained it to me, so I wouldn't expect most users
> to know either.  But as it's easy to explain that probably doesn't
> matter much.

I do not think "the mode is called the overlay mode" is so accurate
a description.  I think I've seen the word 'overlay' used to
describe the behaviour in earlier discussions, but because there is
no 'non-overlay' mode exists in versions of 'git checkout' the
end-users have, the users won't even be aware of the possibility
that mode different from what they are used to see could exist, or
that the mode that they are used to see could be called/explained as
the 'overlay' mode.  IOW, we should pick the best phrase to explain
the behaviour we can use when coming up with the command line
option, and that phrase does not have to be 'overlay'---there is no
"using the word 'overlay' for this is good because the users are
familiar with the existing use of the word", simply because there
isn't such familiarilty ;-)

^ permalink raw reply

* Re: [PATCH] rebase -i: introduce the 'test' command
From: Johannes Schindelin @ 2018-12-02 19:48 UTC (permalink / raw)
  To: Jeff King; +Cc: Paul Morelle, Git Users
In-Reply-To: <20181201200209.GC29120@sigill.intra.peff.net>

Hi Peff,

On Sat, 1 Dec 2018, Jeff King wrote:

> On Thu, Nov 29, 2018 at 09:32:48AM +0100, Johannes Schindelin wrote:
> 
> > > > Would it not make more sense to add a command-line option (and a config
> > > > setting) to re-schedule failed `exec` commands? Like so:
> > > 
> > > Your proposition would do in most cases, however it is not possible to
> > > make a distinction between reschedulable and non-reschedulable commands.
> > 
> > True. But I don't think that's so terrible.
> > 
> > What I think is something to avoid is two commands that do something very,
> > very similar, but with two very, very different names.
> > 
> > In reality, I think that it would even make sense to change the default to
> > reschedule failed `exec` commands. Which is why I suggested to also add a
> > config option.
> 
> I sometimes add "x false" to the top of the todo list to stop and create
> new commits before the first one. That would be awkward if I could never
> get past that line. However, I think elsewhere a "pause" line has been
> discussed, which would serve the same purpose.

Yep, `break`, as Eric pointed out.

After all, you did not really want a command to fail, you just wanted the
interactive rebase to give you a break.

> I wonder how often this kind of "yes, I know it fails, but keep going
> anyway" situation would come up. And what the interface is like for
> getting past it. E.g., what if you fixed a bunch of stuff but your tests
> still fail? You may not want to abandon the changes you've made, but you
> need to "rebase --continue" to move forward. I encounter this often when
> the correct fix is actually in an earlier commit than the one that
> yields the test failure. You can't rewind an interactive rebase, so I
> complete and restart it, adding an "e"dit at the earlier commit.
> 
> How would I move past the test that fails to continue? I guess "git
> rebase --edit-todo" and then manually remove it (and any other remaining
> test lines)?

Yes, the current way would be to use `git rebase --edit-todo`.

> That's not too bad, but I wonder if people would find it more awkward
> than the current way (which is to just "rebase --continue" until you get
> to the end).
> 
> I dunno. I am not sure if I am for or against changing the default, so
> these are just some musings. :)

It's good that you chimed in with your side of things. If you missed the
`break` command, so will many others have missed it. And continue to miss
it.

Besides, Junio mentioned elsewhere that he is in the camp of people who
wait for enough users to complain why some config option isn't the default
to actually change the default.

So... I guess we'll leave the default where it is for now.

Ciao,
Dscho

^ permalink raw reply

* Re: BUG: CR marker ^M doesn't show up in '-' lines of diffs when the ending of the removed line is CR+LF
From: Johannes Sixt @ 2018-12-02 21:22 UTC (permalink / raw)
  To: Frank Schäfer; +Cc: Junio C Hamano, brian m. carlson, git
In-Reply-To: <7a4ecc75-2dc4-041b-3d12-46cddae0a27f@googlemail.com>

Am 02.12.18 um 20:31 schrieb Frank Schäfer:
> Am 29.11.18 um 03:11 schrieb Junio C Hamano:
> [...]
>> This was misspoken a bit.  Let's revise it to
>>
>>   	When producing a colored output (not limited to whitespace
>>   	error coloring of diff output) for contents that are not
>>   	marked as eol=crlf (and other historical means), insert
>>   	<RESET> before a CR that comes immediately before a LF.
> You mean
>       ...
>       <RESET> *after* a CR that comes immediately before a LF."
> 
> 
> OK, AFAICS this produces the desired output in all cases if eol=lf.
> 
> Now what about the case eol=crlf ?
> Keeping the current behavior of '-' lines is correct.
> But shouldn't ^M now be suppressed in '+' lines for a consistent behavior ?

That can be achieved with whitespace=cr-at-eol.

> With other words:
> "If CR comes immediately before a LF, do the following with *all* lines:
> <RESET> after the CR if eol=lf but do not <RESET> after the CR if eol=crlf."

Why? It is the pager's duty to highlight CR, IMO. If it doesn't, but the 
user wants to see them, then they are using the wrong pager or the wrong 
pager settings.

As far as I am concerned, I do not have any of my files marked as 
eol=crlf, but I do *not* want to see ^M in the pager. I.e., having git 
insert <RESET> between CR and LF would do the wrong thing for me.

-- Hannes

^ permalink raw reply

* Re: [PATCH] Do not fail test if '.' is part of $PATH
From: Junio C Hamano @ 2018-12-03  0:29 UTC (permalink / raw)
  To: Jeff King; +Cc: H.Merijn Brand, git
In-Reply-To: <20181201193822.GA28918@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> Since the test is ultimately checking "can we run should-not-run from
> the current directory", might it be simpler to actually try that as the
> precondition? I.e., something like:
> ...

A nice egg of columbus.  It also would save us from mischievous
users who have should-not-run somewhere no the $PATH that outputs
the string we expect (no, I do not think it is a common thing to do;
I am just saying that the solution covers such an extremely stupid
case without special casing).




^ permalink raw reply

* Re: [PATCH] Do not fail test if '.' is part of $PATH
From: Junio C Hamano @ 2018-12-03  1:00 UTC (permalink / raw)
  To: H.Merijn Brand, Jeff King; +Cc: git
In-Reply-To: <20181201180757.0b2d3c89@pc09.procura.nl>

"H.Merijn Brand" <h.m.brand@xs4all.nl> writes:

> When $PATH contains the current directory as .:PATH, PATH:., PATH:.:PATH,
> or (maybe worse) as :PATH, PATH:, or PATH::PATH - as an empty entry is
> identical to having dot in $PATH - this test used to fail

It is totally unclear what "this test" refers to.  Let's retitle it
to

> Subject: [PATCH] t0061: do not fail test if '.' is part of $PATH

and do something like this:

    t0061 created a script named with an unlikely name in the
    current directory to ensure that it is not found via the
    run_command() API, expecting that $PATH does not contain an
    element that names the current directory (i.e. '.' or '') in a
    sane environment.  This obviously would not work if the $PATH
    does contain such an element.

    Introduce a DOT_IN_PATH lazy prerequisite to catch such a case
    and skip the test when the environment is not so sane.

> +test_lazy_prereq DOT_IN_PATH '
> +       case ":$PATH:" in
> +       *:.:*|*::*) true  ;;
> +       *)          false ;;
> +       esac
> +'
> +
> +test_expect_success !DOT_IN_PATH 'run_command is restricted to PATH' '
>         write_script should-not-run <<-\EOF &&
>         echo yikes
>         EOF

I also like Peff's more straight-forward approach that avoids
looking into PATH but instead ask the shell what we care about
(i.e. would we end up running 'should-not-run' if we asked the
system to run it without giving an explicit path to it?).  The last
paragraph of the above would need to change if we were to go in that
direction to something like

    Check if the running shell picks up the script without an
    explicit path to it and skip the test when it does.

perhaps.  The code to do so got a bit more compact than what Peff
wrote but I think it still retains its main beauty, which is how
straight-forward it is.

 t/t0061-run-command.sh | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh
index cf932c8514..17b560370e 100755
--- a/t/t0061-run-command.sh
+++ b/t/t0061-run-command.sh
@@ -29,7 +29,15 @@ test_expect_success 'run_command can run a command' '
 	test_must_be_empty err
 '
 
-test_expect_success 'run_command is restricted to PATH' '
+
+test_lazy_prereq RUNS_COMMANDS_FROM_PWD '
+	write_script runs-commands-from-pwd <<-\EOF &&
+	true
+	EOF
+	runs-commands-from-pwd >/dev/null 2>&1
+'
+
+test_expect_success !RUNS_COMMANDS_FROM_PWD 'run_command is restricted to PATH' '
 	write_script should-not-run <<-\EOF &&
 	echo yikes
 	EOF

^ permalink raw reply related

* Re: BUG: CR marker ^M doesn't show up in '-' lines of diffs when the ending of the removed line is CR+LF
From: Junio C Hamano @ 2018-12-03  1:15 UTC (permalink / raw)
  To: Frank Schäfer; +Cc: Johannes Sixt, brian m. carlson, git
In-Reply-To: <7a4ecc75-2dc4-041b-3d12-46cddae0a27f@googlemail.com>

Frank Schäfer <fschaefer.oss@googlemail.com> writes:

> Hi Junio,
>
> Am 29.11.18 um 03:11 schrieb Junio C Hamano:
> [...]
>> This was misspoken a bit.  Let's revise it to
>>
>>  	When producing a colored output (not limited to whitespace
>>  	error coloring of diff output) for contents that are not
>>  	marked as eol=crlf (and other historical means), insert
>>  	<RESET> before a CR that comes immediately before a LF.
> You mean
>      ...
>      <RESET> *after* a CR that comes immediately before a LF."
>
> OK, AFAICS this produces the desired output in all cases if eol=lf.

OK, yeah, I think I meant "after", i.e. ... CR <RESET> LF, in order
to force CR to be separated from LF.

> Now what about the case eol=crlf ?

I have no strong opinions, other than that "LF in repository, CRLF
in working tree" would make the issue go away (when it is solved for
EOL=LF like the above, that is).

> Keeping the current behavior of '-' lines is correct.
> But shouldn't ^M now be suppressed in '+' lines for a consistent behavior ?

If "LF in repository, CRLF in working tree" is done, there would not
be ^M at the end of the line, not just for removed lines, but also
for added lines, no?

^ permalink raw reply

* Re: [PATCH] t5004: avoid using tar for empty packages
From: Junio C Hamano @ 2018-12-03  1:29 UTC (permalink / raw)
  To: Carlo Marcelo Arenas Belón; +Cc: git
In-Reply-To: <20181202024003.65103-1-carenas@gmail.com>

Carlo Marcelo Arenas Belón  <carenas@gmail.com> writes:

> ea2d20d4c2 ("t5004: avoid using tar for checking emptiness of archive",
> 2013-05-09), introduced a fake empty tar archive to allow for portable
> tests of emptiness without having to invoke tar
>
> 4318094047 ("archive: don't add empty directories to archives", 2017-09-13)
> changed the expected result for its tests from one containing an empty
> directory to a plain empty archive but the portable test wasn't updated
> resulting on them failing again in (at least) NetBSD and OpenBSD
>
> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
> ---
>  t/t5004-archive-corner-cases.sh | 17 ++++++++---------
>  1 file changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/t/t5004-archive-corner-cases.sh b/t/t5004-archive-corner-cases.sh
> index ced44355ca..271eb5a1fd 100755
> --- a/t/t5004-archive-corner-cases.sh
> +++ b/t/t5004-archive-corner-cases.sh
> @@ -3,8 +3,12 @@
>  test_description='test corner cases of git-archive'
>  . ./test-lib.sh
>  
> -test_expect_success 'create commit with empty tree' '
> -	git commit --allow-empty -m foo
> +# the 10knuls.tar file is used to test for an empty git generated tar
> +# without having to invoke tar because an otherwise valid empty GNU tar
> +# will be considered broken by {Open,Net}BSD tar
> +test_expect_success 'create commit with empty tree and fake empty tar' '
> +	git commit --allow-empty -m foo &&
> +	perl -e "print \"\\0\" x 10240" >10knuls.tar
>  '

OK, so you moved the creation of the file with block of NULs to the
set-up phase of the entire script.

>  # Make a dir and clean it up afterwards
> @@ -47,7 +51,6 @@ test_expect_success HEADER_ONLY_TAR_OK 'tar archive of commit with empty tree' '
>  
>  test_expect_success 'tar archive of empty tree is empty' '
>  	git archive --format=tar HEAD: >empty.tar &&
> -	perl -e "print \"\\0\" x 10240" >10knuls.tar &&
>  	test_cmp_bin 10knuls.tar empty.tar
>  '

And because of that, this one that was added for ea2d20d4 ("t5004:
avoid using tar for checking emptiness of archive", 2013-05-09) is
now simplified.  It can use the one that was created in the set-up
phase.

> @@ -106,16 +109,12 @@ test_expect_success 'create a commit with an empty subtree' '
>  
>  test_expect_success 'archive empty subtree with no pathspec' '
>  	git archive --format=tar $root_tree >subtree-all.tar &&
> -	make_dir extract &&
> -	"$TAR" xf subtree-all.tar -C extract &&
> -	check_dir extract
> +	test_cmp_bin 10knuls.tar subtree-all.tar
>  '

And then we avoid the test that assumes that an empty tar archive
can safely and portably extracted, and instead check the emptiness
the same way as the earlier test here ...

>  test_expect_success 'archive empty subtree by direct pathspec' '
>  	git archive --format=tar $root_tree -- sub >subtree-path.tar &&
> -	make_dir extract &&
> -	"$TAR" xf subtree-path.tar -C extract &&
> -	check_dir extract
> +	test_cmp_bin 10knuls.tar subtree-path.tar
>  '

... and here, too.

OK, and the result is consistent with the "We can help GNU and BSD
tar, but NetBSD tar cannot be salvageable" approach, laid out in the
earlier ea2d20d4 ("t5004: avoid using tar for checking emptiness of
archive", 2013-05-09).  Makes sense.

Thanks.

^ permalink raw reply

* Re: [RFC] git clean --local
From: Cameron Boehmer @ 2018-12-03  7:40 UTC (permalink / raw)
  To: gitster; +Cc: rsbecker, avarab, git
In-Reply-To: <xmqqk1kriuu8.fsf@gitster-ct.c.googlers.com>

> > Would something like git clean --exclude=file-pattern work as a
> > compromise notion? Files matching the pattern would not be cleaned
> > regardless of .gitignore or their potential preciousness status
> > long-term. Multiple repetitions of the --exclude option might be
> > supportable. I could see that being somewhat useful in scripting.
>
> I think "git clean" already takes "-e", but I am not sure if it is
> meant to do what you wrote above.

It does already take --exclude=file-pattern, which is like adding
lines to .gitignore. (W/o -x/-X, that would mean DON'T clean them, but
with -x/-X, it means DO clean them.)

>
> If "git clean" takes a pathspec, perhaps you can give a negative
> pathspec to exclude whatever you do not want to get cleaned,
> something like
>
>         git clean '*.o' ':!precious.o'
>

I like this, but I'm also 100% satisfied with Junio's previous suggestion:
    git -c core.excludesFile=/dev/null clean ...

^ permalink raw reply

* Re: [PATCH] format-patch: do not let its diff-options affect --range-diff (was Re: [PATCH 2/2] format-patch: allow for independent diff & range-diff options)
From: Martin Ågren @ 2018-12-03 13:27 UTC (permalink / raw)
  To: Eric Sunshine
  Cc: Junio C Hamano, Johannes Schindelin,
	Ævar Arnfjörð Bjarmason, Git Mailing List
In-Reply-To: <CAPig+cS4peHDOz0cK7RJEHb6ch0+czvau_aGTD1j6xM23G0-pQ@mail.gmail.com>

On Fri, 30 Nov 2018 at 10:32, Eric Sunshine <sunshine@sunshineco.com> wrote:
>
> On Thu, Nov 29, 2018 at 11:27 PM Junio C Hamano <gitster@pobox.com> wrote:
> > Junio C Hamano <gitster@pobox.com> writes:
> > So how about doing this on top of 'master' instead?  As this leaks
> > *no* information wrt how range-diff machinery should behave from the
> > format-patch side by not passing any diffopt, as long as the new
> > code I added to show_range_diff() comes up with a reasonable default
> > diffopts (for which I really would appreciate extra sets of eyes to
> > make sure), this change by definition cannot be wrong (famous last
> > words).

> Another benefit of calling show_range_diff() directly is that when
> "git format-patch --stdout --range-diff=..." is sent to the terminal,
> the range-diff gets colored output for free. I'm pleased to see that
> that still works after this change.

(If the patch below makes any sense to you and you know more about this
diff/color thing, the fourth bullet in the log message below might
interest you.)

> > diff --git a/range-diff.h b/range-diff.h
> > @@ -5,6 +5,11 @@
> > +/*
> > + * Compare series of commmits in RANGE1 and RANGE2, and emit to the
> > + * standard output.  NULL can be passed to DIFFOPT to use the built-in
> > + * default.
> > + */
>
> It is more correct to say that the range-diff is emitted to
> diffopt->file (which may be stdout).

This seems to be an important remark. There's a pretty bad regression
here since when `diffopt` is NULL, we've lost our original, intended
`diffopt->file` and the range-diff ends up on stdout.

Here's my whitespace-damaged WIP. I would be able to pick this up again
in about 6h, but anyone is more than welcome to pick this up and run
with it in the meantime.

This is not a corner of the code that I'm particularly familiar with...

-->8--
Subject: [PATCH] range-diff: always pass at least minimal diff options
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Commit d8981c3f88 ("format-patch: do not let its diff-options affect
--range-diff", 2018-11-30) taught `show_range_diff()` to accept a
NULL-pointer as an indication that it should use its own "reasonable
default". That fixed a regression from a5170794 ("Merge branch
'ab/range-diff-no-patch'", 2018-11-18), but unfortunately it introduced
a regression of its own.

In particular, it means we forget the `file` member of the diff options,
so rather than placing a range-diff in the cover-letter, we write it to
stdout. In order to fix this, rewrite the two callers adjusted by
d8981c3f88 to create a "dummy" set of diff options where they only fill
in which file to use.

A couple of remarks about this commit:

  * No tests. The change in builtin/log.c has been tested manually, the
    one in log-tree.c not at all, other than by running existing tests.

  * I have not convinced myself 100% that there aren't other things that
    are just as important as `file` to pass down.

  * `show_range_diff()` can still take NULL, although that is now dead
    code. If something like this here commit is deemed the proper fix
    for this, that code path could also go, either as part of this
    commit, or separately, once we've cut 2.20.

  * The range-diff is written colored regardless of destination, i.e.,
    when written to a file, it contains garbage.

Signed-off-by: Martin Ågren <martin.agren@gmail.com>
---
 builtin/log.c | 12 +++++++++++-
 log-tree.c    | 12 +++++++++++-
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/builtin/log.c b/builtin/log.c
index 5ac18e2848..0609e41ae5 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1094,9 +1094,19 @@ static void make_cover_letter(struct rev_info
*rev, int use_stdout,
     }

     if (rev->rdiff1) {
+        /**
+         * (At least for now) we only want to pass down
+         * the file handle where we want the range-diff
+         * to appear. Avoid any other diff options until
+         * we know how we want to handle them.
+         */
+        struct diff_options opts;
+        diff_setup(&opts);
+        opts.file = rev->diffopt.file;
+        diff_setup_done(&opts);
         fprintf_ln(rev->diffopt.file, "%s", rev->rdiff_title);
         show_range_diff(rev->rdiff1, rev->rdiff2,
-                rev->creation_factor, 1, NULL);
+                rev->creation_factor, 1, &opts);
     }
 }

diff --git a/log-tree.c b/log-tree.c
index b243779a0b..bc355a4e91 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -755,14 +755,24 @@ void show_log(struct rev_info *opt)

     if (cmit_fmt_is_mail(ctx.fmt) && opt->rdiff1) {
         struct diff_queue_struct dq;
+        struct diff_options opts;

         memcpy(&dq, &diff_queued_diff, sizeof(diff_queued_diff));
         DIFF_QUEUE_CLEAR(&diff_queued_diff);

         next_commentary_block(opt, NULL);
         fprintf_ln(opt->diffopt.file, "%s", opt->rdiff_title);
+        /**
+         * (At least for now) we only want to pass down
+         * the file handle where we want the range-diff
+         * to appear. Avoid any other diff options until
+         * we know how we want to handle them.
+        */
+        diff_setup(&opts);
+        opts.file = opt->diffopt.file;
+        diff_setup_done(&opts);
         show_range_diff(opt->rdiff1, opt->rdiff2,
-                opt->creation_factor, 1, NULL);
+                opt->creation_factor, 1, &opts);

         memcpy(&diff_queued_diff, &dq, sizeof(diff_queued_diff));
     }
-- 
2.20.0.rc2

^ permalink raw reply related

* Re: [PATCH] rebase -i: introduce the 'test' command
From: Phillip Wood @ 2018-12-03 14:31 UTC (permalink / raw)
  To: Jeff King, Johannes Schindelin; +Cc: Paul Morelle, Git Users
In-Reply-To: <20181201200209.GC29120@sigill.intra.peff.net>

On 01/12/2018 20:02, Jeff King wrote:
> On Thu, Nov 29, 2018 at 09:32:48AM +0100, Johannes Schindelin wrote:
> 
>>>> Would it not make more sense to add a command-line option (and a config
>>>> setting) to re-schedule failed `exec` commands? Like so:
>>>
>>> Your proposition would do in most cases, however it is not possible to
>>> make a distinction between reschedulable and non-reschedulable commands.
>>
>> True. But I don't think that's so terrible.
>>
>> What I think is something to avoid is two commands that do something very,
>> very similar, but with two very, very different names.
>>
>> In reality, I think that it would even make sense to change the default to
>> reschedule failed `exec` commands. Which is why I suggested to also add a
>> config option.
> 
> I sometimes add "x false" to the top of the todo list to stop and create
> new commits before the first one. That would be awkward if I could never
> get past that line. However, I think elsewhere a "pause" line has been
> discussed, which would serve the same purpose.
> 
> I wonder how often this kind of "yes, I know it fails, but keep going
> anyway" situation would come up. And what the interface is like for
> getting past it. E.g., what if you fixed a bunch of stuff but your tests
> still fail? You may not want to abandon the changes you've made, but you
> need to "rebase --continue" to move forward. I encounter this often when
> the correct fix is actually in an earlier commit than the one that
> yields the test failure. You can't rewind an interactive rebase, so I
> complete and restart it, adding an "e"dit at the earlier commit.
> 
> How would I move past the test that fails to continue? I guess "git
> rebase --edit-todo" and then manually remove it (and any other remaining
> test lines)?

Perhaps we could teach git rebase --skip to skip a rescheduled command, 
it could be useful if people want to skip rescheduled picks as well 
(though I don't think I've ever had that happen in the wild). I can see 
myself turning on the rescheduling config setting but occasionally 
wanting to be able to skip over the rescheduled exec command.


Best Wishes

Phillip

> That's not too bad, but I wonder if people would find it more awkward
> than the current way (which is to just "rebase --continue" until you get
> to the end).
> 
> I dunno. I am not sure if I am for or against changing the default, so
> these are just some musings. :)
> 
> -Peff
> 


^ permalink raw reply

* Re: [PATCH] rebase -i: introduce the 'test' command
From: Luc Van Oostenryck @ 2018-12-03 17:27 UTC (permalink / raw)
  To: Jeff King; +Cc: Johannes Schindelin, Paul Morelle, Git Users
In-Reply-To: <20181201200209.GC29120@sigill.intra.peff.net>

On Sat, Dec 01, 2018 at 03:02:09PM -0500, Jeff King wrote:
> On Thu, Nov 29, 2018 at 09:32:48AM +0100, Johannes Schindelin wrote:
> 
> > > > Would it not make more sense to add a command-line option (and a config
> > > > setting) to re-schedule failed `exec` commands? Like so:
> > > 
> > > Your proposition would do in most cases, however it is not possible to
> > > make a distinction between reschedulable and non-reschedulable commands.
> > 
> > True. But I don't think that's so terrible.
> > 
> > What I think is something to avoid is two commands that do something very,
> > very similar, but with two very, very different names.
> > 
> > In reality, I think that it would even make sense to change the default to
> > reschedule failed `exec` commands. Which is why I suggested to also add a
> > config option.
> 
> I sometimes add "x false" to the top of the todo list to stop and create
> new commits before the first one. That would be awkward if I could never
> get past that line. However, I think elsewhere a "pause" line has been
> discussed, which would serve the same purpose.
> 
> I wonder how often this kind of "yes, I know it fails, but keep going
> anyway" situation would come up. And what the interface is like for
> getting past it. E.g., what if you fixed a bunch of stuff but your tests
> still fail? You may not want to abandon the changes you've made, but you
> need to "rebase --continue" to move forward. I encounter this often when
> the correct fix is actually in an earlier commit than the one that
> yields the test failure. You can't rewind an interactive rebase, so I
> complete and restart it, adding an "e"dit at the earlier commit.

In this sort of situation, I often whish to be able to do nested rebases.
Even more because it happen relatively often that I forget that I'm
working in a rebase and not on the head, and then it's quite natural
to me to type things like 'git rebase -i @^^^' while already rebasing.
But I suppose this has already been discussed.

-- Luc

^ permalink raw reply

* [PATCH] rebase docs: fix incorrect format of the section Behavioral Differences
From: Johannes Sixt @ 2018-12-03 17:34 UTC (permalink / raw)
  To: Git Mailing List

The text body of section Behavioral Differences is typeset as code,
but should be regular text. Remove the indentation to achieve that.

While here, prettify the language:

- use "the x backend" instead of "x-based rebase";
- use present tense instead of future tense;

and use subsections instead of a list.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
Cf. https://git-scm.com/docs/git-rebase#_behavioral_differences

I actually did not test the result, because I don't have the
infrastructure.

The sentence "The am backend sometimes does not" is not very useful,
but is not my fault ;) It would be great if it could be made more
specific, but I do not know the details.

 Documentation/git-rebase.txt | 30 +++++++++++++++++-------------
 1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 80793bad8d..dff17b3178 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -550,24 +550,28 @@ Other incompatible flag pairs:
 BEHAVIORAL DIFFERENCES
 -----------------------
 
- * empty commits:
+There are some subtle differences how the backends behave.
 
-    am-based rebase will drop any "empty" commits, whether the
-    commit started empty (had no changes relative to its parent to
-    start with) or ended empty (all changes were already applied
-    upstream in other commits).
+Empty commits
+~~~~~~~~~~~~~
+
+The am backend drops any "empty" commits, regardless of whether the
+commit started empty (had no changes relative to its parent to
+start with) or ended empty (all changes were already applied
+upstream in other commits).
 
-    merge-based rebase does the same.
+The merge backend does the same.
 
-    interactive-based rebase will by default drop commits that
-    started empty and halt if it hits a commit that ended up empty.
-    The `--keep-empty` option exists for interactive rebases to allow
-    it to keep commits that started empty.
+The interactive backend drops commits by default that
+started empty and halts if it hits a commit that ended up empty.
+The `--keep-empty` option exists for the interactive backend to allow
+it to keep commits that started empty.
 
-  * directory rename detection:
+Directory rename detection
+~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-    merge-based and interactive-based rebases work fine with
-    directory rename detection.  am-based rebases sometimes do not.
+The merge and interactive backends work fine with
+directory rename detection.  The am backend sometimes does not.
 
 include::merge-strategies.txt[]
 
-- 
2.19.1.1133.g2dd3d172d2

^ permalink raw reply related

* Re: [PATCH] rebase -i: introduce the 'test' command
From: Duy Nguyen @ 2018-12-03 17:53 UTC (permalink / raw)
  To: Jeff King, Johannes Schindelin; +Cc: Paul Morelle, Git Users
In-Reply-To: <20181201200209.GC29120@sigill.intra.peff.net>

On Sat, Dec 01, 2018 at 03:02:09PM -0500, Jeff King wrote:
> I sometimes add "x false" to the top of the todo list to stop and create
> new commits before the first one.

And here I've been doing the same by "edit" the first commit, add a
new commit then reorder them in the second interactive rebase :P

This made me look at git-rebase.txt to really learn about interactive
rebase. I think the interactive rebase section could use some
improvements. Its style looks.. umm.. more story telling than a
reference. Perhaps something like this to at least highlight the
commands.

-- 8< --
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 80793bad8d..c569b3370b 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -637,22 +637,22 @@ The oneline descriptions are purely for your pleasure; 'git rebase' will
 not look at them but at the commit names ("deadbee" and "fa1afe1" in this
 example), so do not delete or edit the names.
 
-By replacing the command "pick" with the command "edit", you can tell
+By replacing the command `pick` with the command `edit`, you can tell
 'git rebase' to stop after applying that commit, so that you can edit
 the files and/or the commit message, amend the commit, and continue
 rebasing.
 
 To interrupt the rebase (just like an "edit" command would do, but without
-cherry-picking any commit first), use the "break" command.
+cherry-picking any commit first), use the `break` command.
 
 If you just want to edit the commit message for a commit, replace the
-command "pick" with the command "reword".
+command "pick" with the command `reword`.
 
-To drop a commit, replace the command "pick" with "drop", or just
+To drop a commit, replace the command "pick" with `drop`, or just
 delete the matching line.
 
 If you want to fold two or more commits into one, replace the command
-"pick" for the second and subsequent commits with "squash" or "fixup".
+"pick" for the second and subsequent commits with `squash` or `fixup`.
 If the commits had different authors, the folded commit will be
 attributed to the author of the first commit.  The suggested commit
 message for the folded commit is the concatenation of the commit
@@ -693,7 +693,7 @@ $ git rebase -i -p --onto Q O
 Reordering and editing commits usually creates untested intermediate
 steps.  You may want to check that your history editing did not break
 anything by running a test, or at least recompiling at intermediate
-points in history by using the "exec" command (shortcut "x").  You may
+points in history by using the `exec` command (shortcut `x`).  You may
 do so by creating a todo list like this one:
 
 -------------------------------------------
-- 8< --
--
Duy

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox