* [PATCH 05/13] gitk: fill in `wvals` as the tags are first processed
From: Michael Haggerty @ 2016-12-19 16:45 UTC (permalink / raw)
To: Paul Mackerras; +Cc: git, Michael Haggerty
In-Reply-To: <cover.1482164633.git.mhagger@alum.mit.edu>
It's no longer, and a little bit more direct, to fill in `wvals` at the
same time as we determine the tag's type.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
gitk | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/gitk b/gitk
index d2e3803..fb2f653 100755
--- a/gitk
+++ b/gitk
@@ -6561,6 +6561,7 @@ proc drawtags {id x xt y1} {
set marks {}
set types {}
+ set wvals {}
set maxtags 3
set maxtagpct 25
set maxwidth [expr {[graph_pane_width] * $maxtagpct / 100}]
@@ -6575,14 +6576,17 @@ proc drawtags {id x xt y1} {
# show just a single "n tags..." tag
lappend types tags
if {$ntags == 1} {
- lappend marks "tag..."
+ set text "tag..."
} else {
- lappend marks [format "%d tags..." $ntags]
+ set text [format "%d tags..." $ntags]
}
+ lappend marks $text
+ lappend wvals [font measure mainfont $text]
} else {
foreach tag $tags {
lappend types tag
lappend marks $tag
+ lappend wvals [font measure mainfont $tag]
}
}
}
@@ -6590,8 +6594,10 @@ proc drawtags {id x xt y1} {
foreach head $idheads($id) {
if {$head eq $mainhead} {
lappend types mainhead
+ lappend wvals [font measure mainfontbold $head]
} else {
lappend types head
+ lappend wvals [font measure mainfont $head]
}
lappend marks $head
}
@@ -6600,6 +6606,7 @@ proc drawtags {id x xt y1} {
foreach other $idotherrefs($id) {
lappend types other
lappend marks $other
+ lappend wvals [font measure mainfont $other]
}
}
if {$marks eq {}} {
@@ -6609,15 +6616,8 @@ proc drawtags {id x xt y1} {
set yt [expr {$y1 - 0.5 * $linespc}]
set yb [expr {$yt + $linespc - 1}]
set xvals {}
- set wvals {}
- foreach tag $marks type $types {
- if {$type eq "mainhead"} {
- set wid [font measure mainfontbold $tag]
- } else {
- set wid [font measure mainfont $tag]
- }
+ foreach wid $wvals {
lappend xvals $xt
- lappend wvals $wid
set xt [expr {$xt + $wid + $extra}]
}
set t [$canv create line $x $y1 [lindex $xvals end] $y1 \
--
2.9.3
^ permalink raw reply related
* [PATCH 08/13] gitk: only change the color of the "remote" part of remote refs
From: Michael Haggerty @ 2016-12-19 16:45 UTC (permalink / raw)
To: Paul Mackerras; +Cc: git, Michael Haggerty
In-Reply-To: <cover.1482164633.git.mhagger@alum.mit.edu>
If a remote-tracking reference has the form
refs/remotes/origin/foo/bar
, then the part of the reference that tells which remote it comes from
is `refs/remotes/origin`. So display such a reference as
remotes/origin/foo/bar
^^^^^^^^^^^^^^^
, where the indicated part is displayed in `$remotebgcolor`. The old
code always split the reference name at its last slash, thus rendering
the above remote-tracking reference as
remotes/origin/foo/bar
^^^^^^^^^^^^^^^^^^^
, which makes no sense at all.
Note that this commit doesn't change the rendering of remote-tracking
references with only two slashes (e.g., `refs/remotes/foo`). Such
references were created by `git-svn` when using its old naming scheme.
They are still rendered like
remotes/foo
^^^^^^^^
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
gitk | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gitk b/gitk
index 51ebaf5..c146a15 100755
--- a/gitk
+++ b/gitk
@@ -6558,7 +6558,7 @@ proc remotereftext {head textName prefixName} {
upvar $textName text
upvar $prefixName prefix
- if {[regexp {^((remotes/(.*/|)).*)} $head match text prefix]} {
+ if {[regexp {^((remotes/([^/]+/|)).*)} $head match text prefix]} {
return 1
} else {
set text $head
--
2.9.3
^ permalink raw reply related
* [PATCH 04/13] gitk: use a type "mainhead" to indicate the main HEAD branch
From: Michael Haggerty @ 2016-12-19 16:45 UTC (permalink / raw)
To: Paul Mackerras; +Cc: git, Michael Haggerty
In-Reply-To: <cover.1482164633.git.mhagger@alum.mit.edu>
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
gitk | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/gitk b/gitk
index 502f0aa..d2e3803 100755
--- a/gitk
+++ b/gitk
@@ -6588,7 +6588,11 @@ proc drawtags {id x xt y1} {
}
if {[info exists idheads($id)]} {
foreach head $idheads($id) {
- lappend types head
+ if {$head eq $mainhead} {
+ lappend types mainhead
+ } else {
+ lappend types head
+ }
lappend marks $head
}
}
@@ -6607,7 +6611,7 @@ proc drawtags {id x xt y1} {
set xvals {}
set wvals {}
foreach tag $marks type $types {
- if {$type eq "head" && $tag eq $mainhead} {
+ if {$type eq "mainhead"} {
set wid [font measure mainfontbold $tag]
} else {
set wid [font measure mainfont $tag]
@@ -6639,11 +6643,11 @@ proc drawtags {id x xt y1} {
set rowtextx([rowofcommit $id]) [expr {$xr + $linespc}]
} else {
# draw a head or other ref
- if {$type eq "head"} {
+ if {$type eq "mainhead"} {
+ set col $headbgcolor
+ set font mainfontbold
+ } elseif {$type eq "head"} {
set col $headbgcolor
- if {$tag eq $mainhead} {
- set font mainfontbold
- }
} else {
set col "#ddddff"
}
@@ -6663,7 +6667,7 @@ proc drawtags {id x xt y1} {
-font $font -tags [list tag.$id text]]
if {$type eq "tag" || $type eq "tags"} {
$canv bind $t <1> $tagclick
- } elseif {$type eq "head"} {
+ } elseif {$type eq "head" || $type eq "mainhead"} {
$canv bind $t $ctxbut [list headmenu %X %Y $id $tag_quoted]
}
}
--
2.9.3
^ permalink raw reply related
* [PATCH 06/13] gitk: simplify regexp
From: Michael Haggerty @ 2016-12-19 16:45 UTC (permalink / raw)
To: Paul Mackerras; +Cc: git, Michael Haggerty
In-Reply-To: <cover.1482164633.git.mhagger@alum.mit.edu>
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
gitk | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gitk b/gitk
index fb2f653..84a5326 100755
--- a/gitk
+++ b/gitk
@@ -6654,7 +6654,7 @@ proc drawtags {id x xt y1} {
set xl [expr {$xl - $delta/2}]
$canv create polygon $x $yt $xr $yt $xr $yb $x $yb \
-width 1 -outline black -fill $col -tags tag.$id
- if {[regexp {^(remotes/.*/|remotes/)} $tag match remoteprefix]} {
+ if {[regexp {^(remotes/(.*/|))} $tag match remoteprefix]} {
set rwid [font measure mainfont $remoteprefix]
set xi [expr {$x + 1}]
set yti [expr {$yt + 1}]
--
2.9.3
^ permalink raw reply related
* [PATCH 07/13] gitk: extract a method `remotereftext`
From: Michael Haggerty @ 2016-12-19 16:45 UTC (permalink / raw)
To: Paul Mackerras; +Cc: git, Michael Haggerty
In-Reply-To: <cover.1482164633.git.mhagger@alum.mit.edu>
The text value that it also provides to its caller is not used yet, but
it will be soon.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
gitk | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/gitk b/gitk
index 84a5326..51ebaf5 100755
--- a/gitk
+++ b/gitk
@@ -6551,6 +6551,22 @@ proc totalwidth {l font extra} {
return $tot
}
+# Set textName to the text that should be shown in the label for the
+# specified head and prefixName to the prefix text that should be
+# highlighted in $remotebgcolor. Return 1 iff $head is a remote head.
+proc remotereftext {head textName prefixName} {
+ upvar $textName text
+ upvar $prefixName prefix
+
+ if {[regexp {^((remotes/(.*/|)).*)} $head match text prefix]} {
+ return 1
+ } else {
+ set text $head
+ set prefix ""
+ return 0
+ }
+}
+
proc drawtags {id x xt y1} {
global idtags idheads idotherrefs mainhead
global linespc lthickness
@@ -6654,7 +6670,7 @@ proc drawtags {id x xt y1} {
set xl [expr {$xl - $delta/2}]
$canv create polygon $x $yt $xr $yt $xr $yb $x $yb \
-width 1 -outline black -fill $col -tags tag.$id
- if {[regexp {^(remotes/(.*/|))} $tag match remoteprefix]} {
+ if {[remotereftext $tag text remoteprefix]} {
set rwid [font measure mainfont $remoteprefix]
set xi [expr {$x + 1}]
set yti [expr {$yt + 1}]
--
2.9.3
^ permalink raw reply related
* [PATCH 12/13] gitk: add a configuration setting `remoterefbgcolor`
From: Michael Haggerty @ 2016-12-19 16:45 UTC (permalink / raw)
To: Paul Mackerras; +Cc: git, Michael Haggerty
In-Reply-To: <cover.1482164633.git.mhagger@alum.mit.edu>
Remote-tracking references are very different than local branches, so it
would be nice to make it possible to distinguish then visually. So allow
the remote reference background color to be configured separately from
the branch reference background color by introducing a new constant,
`remoterefbgcolor`. For the moment, leave it set to the old default
`headbgcolor`.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
gitk | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/gitk b/gitk
index 0bd4aa5..cb5c715 100755
--- a/gitk
+++ b/gitk
@@ -6574,7 +6574,7 @@ proc drawtags {id x xt y1} {
global linespc lthickness
global canv rowtextx curview fgcolor bgcolor ctxbut
global headbgcolor headfgcolor headoutlinecolor
- global remotebgcolor otherrefbgcolor
+ global remotebgcolor remoterefbgcolor otherrefbgcolor
global tagbgcolor tagfgcolor tagoutlinecolor
global reflinecolor
@@ -6669,8 +6669,10 @@ proc drawtags {id x xt y1} {
if {$type eq "mainhead"} {
set col $headbgcolor
set font mainfontbold
- } elseif {$type eq "head" || $type eq "remote"} {
+ } elseif {$type eq "head"} {
set col $headbgcolor
+ } elseif {$type eq "remote"} {
+ set col $remoterefbgcolor
} else {
set col $otherrefbgcolor
}
@@ -12362,6 +12364,7 @@ set headbgcolor "#00ff00"
set headfgcolor black
set headoutlinecolor black
set remotebgcolor #ffddaa
+set remoterefbgcolor #00ff00
set otherrefbgcolor #ddddff
set tagbgcolor yellow
set tagfgcolor black
@@ -12420,7 +12423,7 @@ set config_variables {
bgcolor fgcolor uifgcolor uifgdisabledcolor colors diffcolors mergecolors
markbgcolor diffcontext selectbgcolor foundbgcolor currentsearchhitbgcolor
extdifftool perfile_attrs headbgcolor headfgcolor headoutlinecolor
- remotebgcolor otherrefbgcolor
+ remotebgcolor remoterefbgcolor otherrefbgcolor
tagbgcolor tagfgcolor tagoutlinecolor reflinecolor
filesepbgcolor filesepfgcolor linehoverbgcolor linehoverfgcolor
linehoveroutlinecolor mainheadcirclecolor workingfilescirclecolor
--
2.9.3
^ permalink raw reply related
* [PATCH 02/13] gitk: keep track of tag types in a separate `types` array
From: Michael Haggerty @ 2016-12-19 16:45 UTC (permalink / raw)
To: Paul Mackerras; +Cc: git, Michael Haggerty
In-Reply-To: <cover.1482164633.git.mhagger@alum.mit.edu>
The resulting code is easier to follow than the old counting-based code,
plus in a moment we will add some more specific types.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
gitk | 37 +++++++++++++++++++++----------------
1 file changed, 21 insertions(+), 16 deletions(-)
diff --git a/gitk b/gitk
index 296efb3..7c830c3 100755
--- a/gitk
+++ b/gitk
@@ -6560,8 +6560,7 @@ proc drawtags {id x xt y1} {
global reflinecolor
set marks {}
- set ntags 0
- set nheads 0
+ set types {}
set singletag 0
set maxtags 3
set maxtagpct 25
@@ -6576,22 +6575,30 @@ proc drawtags {id x xt y1} {
[totalwidth $tags mainfont $extra] > $maxwidth} {
# show just a single "n tags..." tag
set singletag 1
+ lappend types tag
if {$ntags == 1} {
lappend marks "tag..."
} else {
lappend marks [format "%d tags..." $ntags]
- set ntags 1
}
} else {
- set marks [concat $marks $tags]
+ foreach tag $tags {
+ lappend types tag
+ lappend marks $tag
+ }
}
}
if {[info exists idheads($id)]} {
- set marks [concat $marks $idheads($id)]
- set nheads [llength $idheads($id)]
+ foreach head $idheads($id) {
+ lappend types head
+ lappend marks $head
+ }
}
if {[info exists idotherrefs($id)]} {
- set marks [concat $marks $idotherrefs($id)]
+ foreach other $idotherrefs($id) {
+ lappend types other
+ lappend marks $other
+ }
}
if {$marks eq {}} {
return $xt
@@ -6601,10 +6608,8 @@ proc drawtags {id x xt y1} {
set yb [expr {$yt + $linespc - 1}]
set xvals {}
set wvals {}
- set i -1
- foreach tag $marks {
- incr i
- if {$i >= $ntags && $i < $ntags + $nheads && $tag eq $mainhead} {
+ foreach tag $marks type $types {
+ if {$type eq "head" && $tag eq $mainhead} {
set wid [font measure mainfontbold $tag]
} else {
set wid [font measure mainfont $tag]
@@ -6616,12 +6621,12 @@ proc drawtags {id x xt y1} {
set t [$canv create line $x $y1 [lindex $xvals end] $y1 \
-width $lthickness -fill $reflinecolor -tags tag.$id]
$canv lower $t
- foreach tag $marks x $xvals wid $wvals {
+ foreach tag $marks type $types x $xvals wid $wvals {
set tag_quoted [string map {% %%} $tag]
set xl [expr {$x + $delta}]
set xr [expr {$x + $delta + $wid + $lthickness}]
set font mainfont
- if {[incr ntags -1] >= 0} {
+ if {$type eq "tag"} {
# draw a tag
set t [$canv create polygon $x [expr {$yt + $delta}] $xl $yt \
$xr $yt $xr $yb $xl $yb $x [expr {$yb - $delta}] \
@@ -6636,7 +6641,7 @@ proc drawtags {id x xt y1} {
set rowtextx([rowofcommit $id]) [expr {$xr + $linespc}]
} else {
# draw a head or other ref
- if {[incr nheads -1] >= 0} {
+ if {$type eq "head"} {
set col $headbgcolor
if {$tag eq $mainhead} {
set font mainfontbold
@@ -6658,9 +6663,9 @@ proc drawtags {id x xt y1} {
}
set t [$canv create text $xl $y1 -anchor w -text $tag -fill $headfgcolor \
-font $font -tags [list tag.$id text]]
- if {$ntags >= 0} {
+ if {$type eq "tag"} {
$canv bind $t <1> $tagclick
- } elseif {$nheads >= 0} {
+ } elseif {$type eq "head"} {
$canv bind $t $ctxbut [list headmenu %X %Y $id $tag_quoted]
}
}
--
2.9.3
^ permalink raw reply related
* [PATCH 00/13] gitk: tweak rendering of remote-tracking references
From: Michael Haggerty @ 2016-12-19 16:44 UTC (permalink / raw)
To: Paul Mackerras; +Cc: git, Michael Haggerty
This patch series changes a bunch of details about how remote-tracking
references are rendered in the commit list of gitk:
* Omit the "remote/" prefix on normal remote-tracking references. They
are already distinguished via their two-tone rendering and (usually)
longer names, and this change saves a lot of visual clutter and
horizontal space.
* Render remote-tracking references that have more than the usual
three slashes like
origin/foo/bar
^^^^^^^
rather than
origin/foo/bar (formerly remotes/origin/foo/bar)
^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^
, where the indicated part is the prefix that is rendered in a
different color. Usually, such a reference represents a remote
branch that contains a slash in its name, so the new split more
accurately portrays the separation between remote name and remote
branch name.
* Introduce a separate constant to specify the background color used
for the branch name part of remote-tracking references, to allow it
to differ from the color used for local branches (which by default
is bright green).
* Change the default background colors for remote-tracking branches to
light brown and brown (formerly they were pale orange and bright
green).
I understand that the colors of pixels on computer screens is an even
more emotional topic that that of bikesheds, so I implemented the last
change as a separate commit, the last one in the series. Feel free to
drop it if you don't want the default color change.
Along the way, I did a bunch of refactoring in the area to make these
changes easier, and introduced a constant for the background color of
"other" references so that it can also be adjusted by users.
(Unfortunately, these colors can only be adjusted by editing the
configuration file. Someday it would be nice to allow them to be
configured via the preferences dialog.)
It's been a while since I've written any Tcl code, so I apologize in
advance for any howlers :-)
This branch applies against the `master` branch in
git://ozlabs.org/~paulus/gitk.
Michael
Michael Haggerty (13):
gitk: when processing tag labels, don't use `marks` as scratch space
gitk: keep track of tag types in a separate `types` array
gitk: use a type "tags" to indicate abbreviated tags
gitk: use a type "mainhead" to indicate the main HEAD branch
gitk: fill in `wvals` as the tags are first processed
gitk: simplify regexp
gitk: extract a method `remotereftext`
gitk: only change the color of the "remote" part of remote refs
gitk: shorten labels displayed for modern remote-tracking refs
gitk: use type "remote" for remote-tracking references
gitk: introduce a constant otherrefbgcolor
gitk: add a configuration setting `remoterefbgcolor`
gitk: change the default colors for remote-tracking references
gitk | 114 ++++++++++++++++++++++++++++++++++++++++++++-----------------------
1 file changed, 76 insertions(+), 38 deletions(-)
--
2.9.3
^ permalink raw reply
* [PATCH 01/13] gitk: when processing tag labels, don't use `marks` as scratch space
From: Michael Haggerty @ 2016-12-19 16:45 UTC (permalink / raw)
To: Paul Mackerras; +Cc: git, Michael Haggerty
In-Reply-To: <cover.1482164633.git.mhagger@alum.mit.edu>
Instead, just append to it as necessary.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
gitk | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/gitk b/gitk
index a14d7a1..296efb3 100755
--- a/gitk
+++ b/gitk
@@ -6570,18 +6570,20 @@ proc drawtags {id x xt y1} {
set extra [expr {$delta + $lthickness + $linespc}]
if {[info exists idtags($id)]} {
- set marks $idtags($id)
- set ntags [llength $marks]
+ set tags $idtags($id)
+ set ntags [llength $tags]
if {$ntags > $maxtags ||
- [totalwidth $marks mainfont $extra] > $maxwidth} {
+ [totalwidth $tags mainfont $extra] > $maxwidth} {
# show just a single "n tags..." tag
set singletag 1
if {$ntags == 1} {
- set marks [list "tag..."]
+ lappend marks "tag..."
} else {
- set marks [list [format "%d tags..." $ntags]]
+ lappend marks [format "%d tags..." $ntags]
+ set ntags 1
}
- set ntags 1
+ } else {
+ set marks [concat $marks $tags]
}
}
if {[info exists idheads($id)]} {
--
2.9.3
^ permalink raw reply related
* Re: [PATCH 0/5] git check-ref-format --stdin --report-errors
From: Ian Jackson @ 2016-12-19 16:22 UTC (permalink / raw)
To: Michael Haggerty; +Cc: git
In-Reply-To: <561c0338-66cd-f806-7b3b-b422f98a1564@alum.mit.edu>
Michael Haggerty writes ("Re: [PATCH 0/5] git check-ref-format --stdin --report-errors"):
> Thanks for your patches. I left some comments about the individual patches.
Thanks for your review.
> I don't know whether this feature will be popular, but it's not a lot of
> code to add it, so it would be OK with me.
Great.
> Especially given that the output is not especially machine-readable, it
> might be more consistent with other commands to call the new feature
> `--verbose` rather than `--report-errors`.
Sure.
> If it is thought likely that scripts will want to leave a pipe open to
> this command and feed it one query at a time, then it would be helpful
> to flush stdout after each reference's result is written. If the
> opposite use case is common (mass processing of refnames), we could
> always add a `--buffer` option like the one that `git cat-file --batch` has.
I think it should be unbuffered by default, so I will make that
change, along with the fixes from your other mails, and resubmit.
Regards,
Ian.
--
Ian Jackson <ijackson@chiark.greenend.org.uk> These opinions are my own.
If I emailed you from an address @fyvzl.net or @evade.org.uk, that is
a private address which bypasses my fierce spamfilter.
^ permalink raw reply
* RE: [PATCH] pack-objects: don't warn about bitmaps on incremental pack
From: David Turner @ 2016-12-19 16:03 UTC (permalink / raw)
To: 'Jeff King'; +Cc: git@vger.kernel.org
In-Reply-To: <20161217040426.7qeixbihiou5mbsl@sigill.intra.peff.net>
> diff --git a/builtin/gc.c b/builtin/gc.c index 069950d0b4..d3c978c765
> 100644
> --- a/builtin/gc.c
> +++ b/builtin/gc.c
> @@ -191,6 +191,11 @@ static void add_repack_all_option(void)
> }
> }
>
> +static void add_repack_incremental_option(void)
> +{
> + argv_array_push(&repack, "--no-write-bitmap-index"); }
> +
> static int need_to_gc(void)
> {
> /*
> @@ -208,7 +213,9 @@ static int need_to_gc(void)
> */
> if (too_many_packs())
> add_repack_all_option();
> - else if (!too_many_loose_objects())
> + else if (too_many_loose_objects())
> + add_repack_incremental_option();
> + else
> return 0;
>
> if (run_hook_le(NULL, "pre-auto-gc", NULL))
Sure, that's fine.
^ permalink raw reply
* (no subject)
From: Matthew Sanchez @ 2016-12-19 15:16 UTC (permalink / raw)
To: git
subscribe git
^ permalink raw reply
* Re: [PATCH v2 01/34] sequencer: support a new action: 'interactive rebase'
From: Johannes Schindelin @ 2016-12-19 14:25 UTC (permalink / raw)
To: Stephan Beyer; +Cc: Junio C Hamano, git, Kevin Daudt, Dennis Kaarsemaker
In-Reply-To: <f0762491-63ca-0814-0005-b2cbdd4dc505@gmx.net>
Hi Stephan,
On Sat, 17 Dec 2016, Stephan Beyer wrote:
> On 12/14/2016 08:29 PM, Junio C Hamano wrote:
> > Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> >> -/* We will introduce the 'interactive rebase' mode later */
> >> static inline int is_rebase_i(const struct replay_opts *opts)
> >> {
> >> - return 0;
> >> + return opts->action == REPLAY_INTERACTIVE_REBASE;
> >> }
> >>
> >> static const char *get_dir(const struct replay_opts *opts)
> >> {
> >> + if (is_rebase_i(opts))
> >> + return rebase_path();
> >> return git_path_seq_dir();
> >> }
> >
> > This obviously makes the assumption made by 39784cd362 ("sequencer:
> > remove useless get_dir() function", 2016-12-07) invalid, where the
> > "remove useless" thought that the callers of this function wants a
> > single answer that does not depend on opts.
> >
> > I'll revert that commit from the sb/sequencer-abort-safety topic (as
> > the topic is in 'next' already) to keep this one. Please holler if
> > that is a mistaken decision.
>
> It seems to be the right decision if one wants to keep the internal-path
> backwards-compatibility of "git rebase -i" (and it seems that Dscho
> wants to keep it).
You make it sound as if I had any choice there. But you probably know as
well as I do that scripted usage of rebase -i relies on the "internal-path
backwards-compatibility", and as one of my stated goals was not to break
anybody's existing rebase -i usage, well, you know.
> However, maintaining more than one directory (like "sequencer" for
> sequencer state and "rebase-merge" for rebase todo and log) can cause
> the necessity to be even more careful when hacking on sequencer... For
> example, the cleanup code must delete both of them, not only one of them.
That is incorrect. It depends on the options which directory is used. And
it is that directory (and not both) that should be cleaned up in the end.
Otherwise you run into a ton of pain e.g. when running a rebase -i with an
`exec git cherry-pick ...` line: all of a sudden, that little innocuous
line would simply destroy the state directory of the current rebase -i.
That's a rather big no-no.
> Hence, I think that it's a wiser choice to keep one directory for
> sequencer state *and* additional files.
See above for a good reason not to choose that.
> If we (a) save everything in "sequencer", we break the internal-path
> backwards-compatbility but we can get rid of get_dir()...
... and we break power users' scripts that use rebase -i. Making those
power users very angry.
Including me.
> If we (b) save everything in "rebase-merge" when using rebase -i and
> save everything in "sequencer" for pick/revert, we are 100%
> backwards-compatible, but we have to change every occurrence of
> git_path_seq_dir() to get_dir()
Yes, that is exactly what we have to do. And that is exactly what I did,
too, in the patch series I labeled "prepare-sequencer".
The only three uses of git_path_seq_dir() I left intact are:
- in builtin/commit.c, where it is used to determine whether a cherry-pick
is really still in progress, after testing that there is a
CHERRY_PICK_HEAD file
- in get_dir() (obviously)
- in create_seq_dir(), where we could technically avoid calling the
function a whopping three times and use a parameter instead (not my
construction site, though, so I leave this alone)
> and the GIT_PATH_FUNC definitions of git_path_{todo,opts,head}_file must
> be replaced by definitions using get_dir().
That is incorrect. The git_path_todo(), git_path_opts() and
git_path_head() functions need to be used in the cherry-pick/revert code
path. So they need to be left as-are.
Ciao,
Dscho
^ permalink raw reply
* RE: Can't get git to stop outputting to StdErr
From: Scott R. Chamberlain @ 2016-12-19 14:56 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git@vger.kernel.org
In-Reply-To: <xmqq7f8syyhg.fsf@gitster.mtv.corp.google.com>
Microsoft fixed their git repo implementation to no longer cause this problem.
https://connect.microsoft.com/VisualStudio/feedback/details/3109654/git-host-on-visualstudio-com-does-not-respect-the-q-flag-for-pushes
Scott Chamberlain
Software Engineer
ImproMed, LLC
(800) 925-7171
www.impromed.com
-----Original Message-----
From: Scott R. Chamberlain
Sent: Friday, October 28, 2016 4:09 PM
To: 'Junio C Hamano' <gitster@pobox.com>
Cc: git@vger.kernel.org
Subject: RE: Can't get git to stop outputting to StdErr
This is talking to a visualstudio.com git repo. I will file a bug with them.
Scott Chamberlain
Software Engineer
ImproMed, LLC
(800) 925-7171
www.impromed.com
-----Original Message-----
From: Junio C Hamano [mailto:gitster@pobox.com]
Sent: Friday, October 28, 2016 4:07 PM
To: Scott R. Chamberlain <srchamberlain@impromed.com>
Cc: git@vger.kernel.org
Subject: Re: Can't get git to stop outputting to StdErr
"Scott R. Chamberlain" <srchamberlain@impromed.com> writes:
> The line I do is:
>
> git push -q binaryRepo HEAD:"$Env:BUILD_SOURCEBRANCH"
This would
(1) squelch the output from the sending side (i.e. local), and
(2) ask "quiet" to the receiving side (i.e. remote), if they know
how to be quiet.
> But I get the following in my log after the build
>
> 2016-10-28T20:05:32.3179442Z ##[error]remote:
> remote: Analyzing objects... (3/3) (657 ms)
> remote: Storing packfile... done (40 ms)
> remote: Storing index... done (42 ms)
These three lines prefixed with "remote:" are coming from the software that runs on the remote machine that accepts your push, but the way it says these three things do not look familiar to me. Is it possible that the remote machine is running a Git server that is not ours, which lacks the support for "quiet" capability? If that is the case, the symptom is understandable.
A quick archive search tells me that you are seeing the same issue as this one:
https://public-inbox.org/git/20160516133731.GA6903@sigill.intra.peff.net/
where the concluding remark, to which I agree, is:
The server side here is clearly not stock git, from the content
of those progress messages (some googling shows it looks like
whatever visualstudio.com is running, but I don't know what that
is). So either the server implementation doesn't support the
"quiet" protocol extension, or it is ignoring it. It might be
worth filing a bug with them.
--
Rely On Us.
ImproMed LLC
Henry Schein Animal Health
--
^ permalink raw reply
* Re: [PATCH] parse-options: print "fatal:" before usage_msg_opt()
From: Duy Nguyen @ 2016-12-19 14:30 UTC (permalink / raw)
To: Jeff King; +Cc: Git Mailing List
In-Reply-To: <20161219140535.ehrbosgn32nl27ki@sigill.intra.peff.net>
On Mon, Dec 19, 2016 at 9:05 PM, Jeff King <peff@peff.net> wrote:
> On Mon, Dec 19, 2016 at 08:53:30PM +0700, Duy Nguyen wrote:
>
>> >> Your commit message does not make clear if you want this "fatal" to be
>> >> grep-able (by scripts) or not. If not, please _() the string. Maybe
>> >> this to reduce work for translators
>> >>
>> >> /* TRANSLATORS: this is the prefix before usage error */
>> >> fprintf(stderr, "%s %s\n\n", _("fatal:"), msg);
>> >
>> > I don't think we translate any of our "fatal:", "error:", etc, do we?
>> > It certainly doesn't look like it in usage.c.
>>
>> I know. But those existed before the l10n starts, some of those belong
>> to plumbing messages. This one is new.
>
> We add lots of new messages which are themselves translated, and they
> still get untranslated prefixes. It seems like consistency is more
> important than translating this one spot. But then, I do not use a
> translated git myself, so I would not see the difference either way.
I'm kinda used to the half English half Vietnamese messages after so
many years (not just git). I do like the prefix translated as well.
But I guess we could leave this a is for now. At least we know the
scope of this message and will have easier time i18n-izing it when we
do the same for die(), warning() and friends.
--
Duy
^ permalink raw reply
* Re: [PATCH] parse-options: print "fatal:" before usage_msg_opt()
From: Jeff King @ 2016-12-19 14:32 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Git Mailing List
In-Reply-To: <CACsJy8AswJb1wzV+mcQ3HXrib3HRtVje7PPd9AWPE9Bi32_2sw@mail.gmail.com>
On Mon, Dec 19, 2016 at 09:30:09PM +0700, Duy Nguyen wrote:
> On Mon, Dec 19, 2016 at 9:05 PM, Jeff King <peff@peff.net> wrote:
> > On Mon, Dec 19, 2016 at 08:53:30PM +0700, Duy Nguyen wrote:
> >
> >> >> Your commit message does not make clear if you want this "fatal" to be
> >> >> grep-able (by scripts) or not. If not, please _() the string. Maybe
> >> >> this to reduce work for translators
> >> >>
> >> >> /* TRANSLATORS: this is the prefix before usage error */
> >> >> fprintf(stderr, "%s %s\n\n", _("fatal:"), msg);
> >> >
> >> > I don't think we translate any of our "fatal:", "error:", etc, do we?
> >> > It certainly doesn't look like it in usage.c.
> >>
> >> I know. But those existed before the l10n starts, some of those belong
> >> to plumbing messages. This one is new.
> >
> > We add lots of new messages which are themselves translated, and they
> > still get untranslated prefixes. It seems like consistency is more
> > important than translating this one spot. But then, I do not use a
> > translated git myself, so I would not see the difference either way.
>
> I'm kinda used to the half English half Vietnamese messages after so
> many years (not just git). I do like the prefix translated as well.
> But I guess we could leave this a is for now. At least we know the
> scope of this message and will have easier time i18n-izing it when we
> do the same for die(), warning() and friends.
Yeah. It's a big enough change that at the very least it should go into
its own patch. I don't have a strong opinion myself, except that if we
want to leave plumbing messages completely grep-able, we probably need
to distinguish between different calls to die(), etc. Which sounds kind
of nasty.
I dunno. I _thought_ nobody was supposed to be grepping stderr, even for
plumbing commands. But maybe that does not match reality.
-Peff
^ permalink raw reply
* Re: [PATCH v2 00/34] Teach the sequencer to act as rebase -i's backend
From: Johannes Schindelin @ 2016-12-19 14:09 UTC (permalink / raw)
To: Johannes Sixt; +Cc: git, Junio C Hamano, Kevin Daudt, Dennis Kaarsemaker
In-Reply-To: <9d399b83-92c6-44e9-8415-7695e690e8be@kdbg.org>
Hi Hannes,
On Wed, 14 Dec 2016, Johannes Sixt wrote:
> Am 13.12.2016 um 16:29 schrieb Johannes Schindelin:
> > base-commit: 8d7a455ed52e2a96debc080dfc011b6bb00db5d2
> > Published-As: https://github.com/dscho/git/releases/tag/sequencer-i-v2
> > Fetch-It-Via: git fetch https://github.com/dscho/git sequencer-i-v2
>
> Thank you so much!
>
> I would appreciate if you could publish a branch that contains the end game so
> that I can test it, too. Currently I am still using
>
> git://github.com/dscho/git interactive-rebase (fca871a3cf4d)
That's the one. Unless I forget, I use the Git garden shears to update
'interactive-rebase' whenever I update 'sequencer-i', too.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH v2 04/34] sequencer (rebase -i): implement the 'exec' command
From: Johannes Schindelin @ 2016-12-19 14:06 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Kevin Daudt, Dennis Kaarsemaker
In-Reply-To: <xmqq7f731pis.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Tue, 13 Dec 2016, Junio C Hamano wrote:
> Please have "else if" on the same line as "}" that closes the
> "if (...) {" in the same if/else if/else cascade.
Fixed. This affected quite a few more patches than just this one, and
added two more to this already large patch series.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] parse-options: print "fatal:" before usage_msg_opt()
From: Jeff King @ 2016-12-19 14:05 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Git Mailing List
In-Reply-To: <CACsJy8BgfyDTUmNh_PvoVcz2q92eNTRZy5myegTdi8mu6imVUQ@mail.gmail.com>
On Mon, Dec 19, 2016 at 08:53:30PM +0700, Duy Nguyen wrote:
> >> Your commit message does not make clear if you want this "fatal" to be
> >> grep-able (by scripts) or not. If not, please _() the string. Maybe
> >> this to reduce work for translators
> >>
> >> /* TRANSLATORS: this is the prefix before usage error */
> >> fprintf(stderr, "%s %s\n\n", _("fatal:"), msg);
> >
> > I don't think we translate any of our "fatal:", "error:", etc, do we?
> > It certainly doesn't look like it in usage.c.
>
> I know. But those existed before the l10n starts, some of those belong
> to plumbing messages. This one is new.
We add lots of new messages which are themselves translated, and they
still get untranslated prefixes. It seems like consistency is more
important than translating this one spot. But then, I do not use a
translated git myself, so I would not see the difference either way.
-Peff
^ permalink raw reply
* Re: [PATCH] parse-options: print "fatal:" before usage_msg_opt()
From: Duy Nguyen @ 2016-12-19 13:53 UTC (permalink / raw)
To: Jeff King; +Cc: Git Mailing List
In-Reply-To: <20161219134148.vxa4fz3jw2i23lfm@sigill.intra.peff.net>
On Mon, Dec 19, 2016 at 8:41 PM, Jeff King <peff@peff.net> wrote:
> On Mon, Dec 19, 2016 at 07:07:19PM +0700, Duy Nguyen wrote:
>
>> On Wed, Dec 14, 2016 at 10:10:10AM -0500, Jeff King wrote:
>> > diff --git a/parse-options.c b/parse-options.c
>> > index 312a85dbd..4fbe924a5 100644
>> > --- a/parse-options.c
>> > +++ b/parse-options.c
>> > @@ -661,7 +661,7 @@ void NORETURN usage_msg_opt(const char *msg,
>> > const char * const *usagestr,
>> > const struct option *options)
>> > {
>> > - fprintf(stderr, "%s\n\n", msg);
>> > + fprintf(stderr, "fatal: %s\n\n", msg);
>>
>> Your commit message does not make clear if you want this "fatal" to be
>> grep-able (by scripts) or not. If not, please _() the string. Maybe
>> this to reduce work for translators
>>
>> /* TRANSLATORS: this is the prefix before usage error */
>> fprintf(stderr, "%s %s\n\n", _("fatal:"), msg);
>
> I don't think we translate any of our "fatal:", "error:", etc, do we?
> It certainly doesn't look like it in usage.c.
I know. But those existed before the l10n starts, some of those belong
to plumbing messages. This one is new.
--
Duy
^ permalink raw reply
* Re: [PATCH v2 03/34] sequencer (rebase -i): implement the 'edit' command
From: Johannes Schindelin @ 2016-12-19 13:46 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Kevin Daudt, Dennis Kaarsemaker
In-Reply-To: <xmqqbmwf1pqd.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Tue, 13 Dec 2016, Junio C Hamano wrote:
> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
>
> > @@ -43,6 +44,20 @@ static GIT_PATH_FUNC(rebase_path_todo, "rebase-merge/git-rebase-todo")
> > */
> > static GIT_PATH_FUNC(rebase_path_author_script, "rebase-merge/author-script")
> > /*
>
> It is minor, but please have a blank line to separate the logical
> blocks.
Ah, but where to draw the line? These comments in front of some of the
rebase_path_* functions clarify the particular role of the corresponding
paths, but all of those functions belong to the same block of
rebase_path_* functions. That latter circumstance is what made me decide
to *not* insert blank lines here, but only a blank line before that block
(to separate from the sequencer path functions) and after that block (to
separate from the rest of the code).
> If you have "comment for thing A" before "thing A", then having a blank
> after that before "comment for thing B" and "thing B" that follow would
> help. Otherwise "thing A" immediately followed by "comment for thing B"
> are (mis)read together, leading to nonsense.
In this case, I think it is quite obvious that the comments belong to the
immediately following line, and that all of the path functions are part of
a bigger block.
> > + * When an "edit" rebase command is being processed, the SHA1 of the
> > + * commit to be edited is recorded in this file. When "git rebase
> > + * --continue" is executed, if there are any staged changes then they
> > + * will be amended to the HEAD commit, but only provided the HEAD
> > + * commit is still the commit to be edited. When any other rebase
> > + * command is processed, this file is deleted.
> > + */
> > +static GIT_PATH_FUNC(rebase_path_amend, "rebase-merge/amend")
> > +/*
> > + * When we stop at a given patch via the "edit" command, this file contains
> > + * the long commit name of the corresponding patch.
>
> If you abbreviate an object name to 38-hex that is still long but
> that is not full; if you meant full 40-hex, better spell it out as
> "full"---that conveys useful information to programmers (e.g. they
> can just use get_sha1_hex()).
>
> But I think you are writing short_commit_name() to it? So perhaps
> "an abbreviated commit object name"?
Fixed.
> > @@ -1301,9 +1318,87 @@ static int save_opts(struct replay_opts *opts)
> > return res;
> > }
> >
> > +static int make_patch(struct commit *commit, struct replay_opts *opts)
> > +{
> > + struct strbuf buf = STRBUF_INIT;
> > + struct rev_info log_tree_opt;
> > + const char *commit_buffer = get_commit_buffer(commit, NULL), *subject, *p;
> > + int res = 0;
> > +
> > + p = short_commit_name(commit);
> > + if (write_message(p, strlen(p), rebase_path_stopped_sha(), 1) < 0)
> > + return -1;
> > +
> > + strbuf_addf(&buf, "%s/patch", get_dir(opts));
> > + memset(&log_tree_opt, 0, sizeof(log_tree_opt));
> > + init_revisions(&log_tree_opt, NULL);
> > + log_tree_opt.abbrev = 0;
> > + log_tree_opt.diff = 1;
> > + log_tree_opt.diffopt.output_format = DIFF_FORMAT_PATCH;
> > + log_tree_opt.disable_stdin = 1;
> > + log_tree_opt.no_commit_id = 1;
> > + log_tree_opt.diffopt.file = fopen(buf.buf, "w");
> > + log_tree_opt.diffopt.use_color = GIT_COLOR_NEVER;
> > + if (!log_tree_opt.diffopt.file)
> > + res |= error_errno(_("could not open '%s'"), buf.buf);
> > + else {
> > + res |= log_tree_commit(&log_tree_opt, commit);
> > + fclose(log_tree_opt.diffopt.file);
> > + }
> > + strbuf_reset(&buf);
> > + strbuf_addf(&buf, "%s/message", get_dir(opts));
> > + if (!file_exists(buf.buf)) {
> > + find_commit_subject(commit_buffer, &subject);
> > + res |= write_message(subject, strlen(subject), buf.buf, 1);
> > + unuse_commit_buffer(commit, commit_buffer);
> > + }
> > + strbuf_release(&buf);
> > +
> > + return res;
> > +}
>
> OK. This seems to match what scripted make_patch does in a handful
> of lines. We probably should have given you a helper to reduce
> boilerplate that sets up log_tree_opt so that this function does not
> have to be this long, but that is a separate topic.
>
> Does it matter output_format is set to FORMAT_PATCH here, though?
> With --no-commit-id set, I suspect there is no log message or
> authorship information given to the output.
>
> As you are only interested in seeing the patch/diff in this file and
> the log is stored in a separate "message" file, as long as "patch"
> file gets the patch correctly, it is not a problem, but it just
> looked strange to see FORMAT_PATCH there.
I am indifferent as to FORMAT_PATCH. It is there simply as a direct
translation of the `diff-tree -p` command in
https://github.com/git/git/blob/v2.11.0/git-rebase--interactive.sh#L188
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 5/5] check-ref-format: New --stdin option
From: Michael Haggerty @ 2016-12-19 13:43 UTC (permalink / raw)
To: Ian Jackson, git
In-Reply-To: <20161104191358.28812-6-ijackson@chiark.greenend.org.uk>
On 11/04/2016 08:13 PM, Ian Jackson wrote:
> Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
> ---
> Documentation/git-check-ref-format.txt | 10 ++++++++--
> builtin/check-ref-format.c | 34 +++++++++++++++++++++++++++++++---
> 2 files changed, 39 insertions(+), 5 deletions(-)
>
> [...]
> diff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c
> index 559d5c2..87f52fa 100644
> --- a/builtin/check-ref-format.c
> +++ b/builtin/check-ref-format.c
> @@ -76,6 +76,7 @@ static int check_one_ref_format(const char *refname)
> int cmd_check_ref_format(int argc, const char **argv, const char *prefix)
> {
> int i;
> + int use_stdin = 0;
>
> if (argc == 2 && !strcmp(argv[1], "-h"))
> usage(builtin_check_ref_format_usage);
> @@ -93,6 +94,8 @@ int cmd_check_ref_format(int argc, const char **argv, const char *prefix)
> check_branch = 1;
> else if (!strcmp(argv[i], "--report-errors"))
> report_errors = 1;
> + else if (!strcmp(argv[i], "--stdin"))
> + use_stdin = 1;
> else
> usage(builtin_check_ref_format_usage);
> }
> @@ -100,8 +103,33 @@ int cmd_check_ref_format(int argc, const char **argv, const char *prefix)
> if (check_branch && (flags || normalize))
> usage(builtin_check_ref_format_usage);
>
> - if (! (i == argc - 1))
> - usage(builtin_check_ref_format_usage);
> + if (!use_stdin) {
> + if (! (i == argc - 1))
> + usage(builtin_check_ref_format_usage);
> +
> + return check_one_ref_format(argv[i]);
> + } else {
> + char buffer[2048];
> + int worst = 0;
>
> - return check_one_ref_format(argv[i]);
> + if (! (i == argc))
> + usage(builtin_check_ref_format_usage);
> +
> + while (fgets(buffer, sizeof(buffer), stdin)) {
> + char *newline = strchr(buffer, '\n');
> + if (!newline) {
> + fprintf(stderr, "%s --stdin: missing final newline or line too long\n", *argv);
> + exit(127);
> + }
> + *newline = 0;
> + int got = check_one_ref_format(buffer);
Another minor point: project policy is not to mix declarations and code.
Please declare `got` at the top of the block.
> + if (got > worst)
> + worst = got;
> + }
> + if (!feof(stdin)) {
> + perror("reading from stdin");
> + exit(127);
> + }
> + return worst;
> + }
> }
>
Michael
^ permalink raw reply
* Re: [PATCH] parse-options: print "fatal:" before usage_msg_opt()
From: Jeff King @ 2016-12-19 13:41 UTC (permalink / raw)
To: Duy Nguyen; +Cc: git
In-Reply-To: <20161219120719.GF24125@ash>
On Mon, Dec 19, 2016 at 07:07:19PM +0700, Duy Nguyen wrote:
> On Wed, Dec 14, 2016 at 10:10:10AM -0500, Jeff King wrote:
> > diff --git a/parse-options.c b/parse-options.c
> > index 312a85dbd..4fbe924a5 100644
> > --- a/parse-options.c
> > +++ b/parse-options.c
> > @@ -661,7 +661,7 @@ void NORETURN usage_msg_opt(const char *msg,
> > const char * const *usagestr,
> > const struct option *options)
> > {
> > - fprintf(stderr, "%s\n\n", msg);
> > + fprintf(stderr, "fatal: %s\n\n", msg);
>
> Your commit message does not make clear if you want this "fatal" to be
> grep-able (by scripts) or not. If not, please _() the string. Maybe
> this to reduce work for translators
>
> /* TRANSLATORS: this is the prefix before usage error */
> fprintf(stderr, "%s %s\n\n", _("fatal:"), msg);
I don't think we translate any of our "fatal:", "error:", etc, do we?
It certainly doesn't look like it in usage.c.
-Peff
^ permalink raw reply
* Re: [PATCH v2 02/34] sequencer (rebase -i): implement the 'noop' command
From: Johannes Schindelin @ 2016-12-19 13:37 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Kevin Daudt, Dennis Kaarsemaker
In-Reply-To: <xmqqfulr1s5z.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Tue, 13 Dec 2016, Junio C Hamano wrote:
> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
>
> > @@ -1292,7 +1316,12 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts)
> > struct todo_item *item = todo_list->items + todo_list->current;
> > if (save_todo(todo_list, opts))
> > return -1;
> > - res = do_pick_commit(item->command, item->commit, opts);
> > + if (item->command <= TODO_REVERT)
> > + res = do_pick_commit(item->command, item->commit,
> > + opts);
> > + else if (item->command != TODO_NOOP)
> > + return error(_("unknown command %d"), item->command);
>
> I wonder if making this a switch() statement is easier to read in
> the longer run. The only thing at this point we are gaining by "not
> only mapping and enum must match, the orders matter" is so that this
> codepath can do the same thing for PICK and REVERT, but these two
> would become more and more minority as we learn more words.
I doubt that this is easier to read. There are essentially three
categories we are handling: exec, comments, and everything else. IMO the
current code is the easiest to understand.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH v2 01/34] sequencer: support a new action: 'interactive rebase'
From: Johannes Schindelin @ 2016-12-19 13:33 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Kevin Daudt, Dennis Kaarsemaker
In-Reply-To: <xmqqk2b31sfs.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Tue, 13 Dec 2016, Junio C Hamano wrote:
> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
>
> > @@ -395,7 +414,10 @@ static int do_recursive_merge(struct commit *base, struct commit *next,
> >
> > if (active_cache_changed &&
> > write_locked_index(&the_index, &index_lock, COMMIT_LOCK))
> > - /* TRANSLATORS: %s will be "revert" or "cherry-pick" */
> > + /*
> > + * TRANSLATORS: %s will be "revert", "cherry-pick" or
> > + * "rebase -i".
> > + */
>
> IIRC, the "TRANSLATORS:" comment has to deviate from our coding
> style due to tool limitation and has to be done like this:
>
> > + /* TRANSLATORS: %s will be "revert", "cherry-pick" or
> > + * "rebase -i".
> > + */
Aargh. I had fixed that in another patch series already, but failed to do
so in this here patch series. Sorry.
Fixed.
> > @@ -1204,6 +1226,9 @@ static int save_todo(struct todo_list *todo_list, struct replay_opts *opts)
> > const char *todo_path = get_todo_path(opts);
> > int next = todo_list->current, offset, fd;
> >
> > + if (is_rebase_i(opts))
> > + next++;
> > +
>
> This is because...? Everybody else counts 0-based while rebase-i
> counts from 1 or something?
No. Since its conception, edit-patch-series.sh (the artist now known as
rebase -i) processed each line of the "todo script" by writing a new
"todo" file, skipping the first line, and appending it to the "done" file.
The sequencer chooses to write the new "insn sheet" after a successful
operation.
This is not an option for rebase -i, as it has many error paths and it was
simply impossible to express "save the todo script in case of failure" in
shell script.
I added a comment to clarify why the current command is not written to
git-rebase-todo.
Ciao,
Dscho
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox