* 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: 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 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
* (no subject)
From: Matthew Sanchez @ 2016-12-19 15:16 UTC (permalink / raw)
To: git
subscribe git
^ 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
* 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
* [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
* [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 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 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 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 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 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 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 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 09/13] gitk: shorten labels displayed for modern remote-tracking 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 reference has the form
refs/remotes/origin/foo
, then use the label
origin/foo
^^^^^^^
, where the indicated part is displayed in `$remotebgcolor`. The old
code would have displayed such a reference as
remotes/origin/foo
^^^^^^^^^^^^^^^
, which wastes horizontal space displaying `remote/` for every remote
reference. However, if a remote-tracking reference has only two slashes,
like
refs/remotes/baz
, render the label the same as before, namely
remotes/baz
^^^^^^^^
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
gitk | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/gitk b/gitk
index c146a15..d22ce5f 100755
--- a/gitk
+++ b/gitk
@@ -6558,7 +6558,9 @@ 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
+ } elseif {[regexp {^((remotes/).*)} $head match text prefix]} {
return 1
} else {
set text $head
@@ -6613,7 +6615,8 @@ proc drawtags {id x xt y1} {
lappend wvals [font measure mainfontbold $head]
} else {
lappend types head
- lappend wvals [font measure mainfont $head]
+ remotereftext $head text remoteprefix
+ lappend wvals [font measure mainfont $text]
}
lappend marks $head
}
@@ -6644,6 +6647,7 @@ proc drawtags {id x xt y1} {
set xl [expr {$x + $delta}]
set xr [expr {$x + $delta + $wid + $lthickness}]
set font mainfont
+ set text $tag
if {$type eq "tag" || $type eq "tags"} {
# draw a tag
set t [$canv create polygon $x [expr {$yt + $delta}] $xl $yt \
@@ -6679,7 +6683,7 @@ proc drawtags {id x xt y1} {
-width 0 -fill $remotebgcolor -tags tag.$id
}
}
- set t [$canv create text $xl $y1 -anchor w -text $tag -fill $headfgcolor \
+ set t [$canv create text $xl $y1 -anchor w -text $text -fill $headfgcolor \
-font $font -tags [list tag.$id text]]
if {$type eq "tag" || $type eq "tags"} {
$canv bind $t <1> $tagclick
--
2.9.3
^ permalink raw reply related
* [PATCH 10/13] gitk: use type "remote" for remote-tracking references
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>
This is clearer, and also lets us avoid calling `remotereftext` a second
time for normal branches.
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 d22ce5f..44f4d70 100755
--- a/gitk
+++ b/gitk
@@ -6613,9 +6613,11 @@ proc drawtags {id x xt y1} {
if {$head eq $mainhead} {
lappend types mainhead
lappend wvals [font measure mainfontbold $head]
+ } elseif {[remotereftext $head text remoteprefix]} {
+ lappend types remote
+ lappend wvals [font measure mainfont $text]
} else {
lappend types head
- remotereftext $head text remoteprefix
lappend wvals [font measure mainfont $text]
}
lappend marks $head
@@ -6666,7 +6668,7 @@ proc drawtags {id x xt y1} {
if {$type eq "mainhead"} {
set col $headbgcolor
set font mainfontbold
- } elseif {$type eq "head"} {
+ } elseif {$type eq "head" || $type eq "remote"} {
set col $headbgcolor
} else {
set col "#ddddff"
@@ -6674,7 +6676,8 @@ 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 {[remotereftext $tag text remoteprefix]} {
+ if {$type eq "remote"} {
+ 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 11/13] gitk: introduce a constant otherrefbgcolor
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>
This is the value used for references other than tags, heads, and
remote-tracking references (e.g., `refs/stash`).
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 44f4d70..0bd4aa5 100755
--- a/gitk
+++ b/gitk
@@ -6573,7 +6573,8 @@ proc drawtags {id x xt y1} {
global idtags idheads idotherrefs mainhead
global linespc lthickness
global canv rowtextx curview fgcolor bgcolor ctxbut
- global headbgcolor headfgcolor headoutlinecolor remotebgcolor
+ global headbgcolor headfgcolor headoutlinecolor
+ global remotebgcolor otherrefbgcolor
global tagbgcolor tagfgcolor tagoutlinecolor
global reflinecolor
@@ -6671,7 +6672,7 @@ proc drawtags {id x xt y1} {
} elseif {$type eq "head" || $type eq "remote"} {
set col $headbgcolor
} else {
- set col "#ddddff"
+ set col $otherrefbgcolor
}
set xl [expr {$xl - $delta/2}]
$canv create polygon $x $yt $xr $yt $xr $yb $x $yb \
@@ -12361,6 +12362,7 @@ set headbgcolor "#00ff00"
set headfgcolor black
set headoutlinecolor black
set remotebgcolor #ffddaa
+set otherrefbgcolor #ddddff
set tagbgcolor yellow
set tagfgcolor black
set tagoutlinecolor black
@@ -12418,7 +12420,8 @@ set config_variables {
bgcolor fgcolor uifgcolor uifgdisabledcolor colors diffcolors mergecolors
markbgcolor diffcontext selectbgcolor foundbgcolor currentsearchhitbgcolor
extdifftool perfile_attrs headbgcolor headfgcolor headoutlinecolor
- remotebgcolor tagbgcolor tagfgcolor tagoutlinecolor reflinecolor
+ remotebgcolor otherrefbgcolor
+ tagbgcolor tagfgcolor tagoutlinecolor reflinecolor
filesepbgcolor filesepfgcolor linehoverbgcolor linehoverfgcolor
linehoveroutlinecolor mainheadcirclecolor workingfilescirclecolor
indexcirclecolor circlecolors linkfgcolor circleoutlinecolor
--
2.9.3
^ permalink raw reply related
* [PATCH 13/13] gitk: change the default colors for remote-tracking references
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 of coloring the reference name part of remote-tracking
references the same bright green as used for local branches, change it
to a subtler brown. Change the remote name color to go better with the
brown.
I chose these colors because
* Remote-tracking references are less important than local branches, so
it is appropriate that their coloring be subtler.
* Brown isn't used elsewhere.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
gitk | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/gitk b/gitk
index cb5c715..be97640 100755
--- a/gitk
+++ b/gitk
@@ -12363,8 +12363,8 @@ set markbgcolor "#e0e0ff"
set headbgcolor "#00ff00"
set headfgcolor black
set headoutlinecolor black
-set remotebgcolor #ffddaa
-set remoterefbgcolor #00ff00
+set remotebgcolor #d9d4b2
+set remoterefbgcolor #c1a677
set otherrefbgcolor #ddddff
set tagbgcolor yellow
set tagfgcolor black
--
2.9.3
^ permalink raw reply related
* [PATCH 03/13] gitk: use a type "tags" to indicate abbreviated tags
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>
This replaces the functionality of the old `singletag` variable.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
gitk | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/gitk b/gitk
index 7c830c3..502f0aa 100755
--- a/gitk
+++ b/gitk
@@ -6561,7 +6561,6 @@ proc drawtags {id x xt y1} {
set marks {}
set types {}
- set singletag 0
set maxtags 3
set maxtagpct 25
set maxwidth [expr {[graph_pane_width] * $maxtagpct / 100}]
@@ -6574,8 +6573,7 @@ proc drawtags {id x xt y1} {
if {$ntags > $maxtags ||
[totalwidth $tags mainfont $extra] > $maxwidth} {
# show just a single "n tags..." tag
- set singletag 1
- lappend types tag
+ lappend types tags
if {$ntags == 1} {
lappend marks "tag..."
} else {
@@ -6626,13 +6624,13 @@ proc drawtags {id x xt y1} {
set xl [expr {$x + $delta}]
set xr [expr {$x + $delta + $wid + $lthickness}]
set font mainfont
- if {$type eq "tag"} {
+ if {$type eq "tag" || $type eq "tags"} {
# draw a tag
set t [$canv create polygon $x [expr {$yt + $delta}] $xl $yt \
$xr $yt $xr $yb $xl $yb $x [expr {$yb - $delta}] \
-width 1 -outline $tagoutlinecolor -fill $tagbgcolor \
-tags tag.$id]
- if {$singletag} {
+ if {$type eq "tags"} {
set tagclick [list showtags $id 1]
} else {
set tagclick [list showtag $tag_quoted 1]
@@ -6663,7 +6661,7 @@ 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 {$type eq "tag"} {
+ if {$type eq "tag" || $type eq "tags"} {
$canv bind $t <1> $tagclick
} elseif {$type eq "head"} {
$canv bind $t $ctxbut [list headmenu %X %Y $id $tag_quoted]
--
2.9.3
^ permalink raw reply related
* Re: [PATCH v2 07/34] sequencer (rebase -i): add support for the 'fixup' and 'squash' commands
From: Johannes Schindelin @ 2016-12-19 16:58 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Kevin Daudt, Dennis Kaarsemaker
In-Reply-To: <xmqqlgvhuj82.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Thu, 15 Dec 2016, Junio C Hamano wrote:
> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
>
> > diff --git a/sequencer.c b/sequencer.c
> > index f6e20b142a..271c21581d 100644
> > --- a/sequencer.c
> > +++ b/sequencer.c
> > @@ -45,6 +45,35 @@ static GIT_PATH_FUNC(rebase_path_todo, "rebase-merge/git-rebase-todo")
> > */
> > static GIT_PATH_FUNC(rebase_path_done, "rebase-merge/done")
> > /*
> > + * The commit message that is planned to be used for any changes that
> > + * need to be committed following a user interaction.
> > + */
> > +static GIT_PATH_FUNC(rebase_path_message, "rebase-merge/message")
> > +/*
> > + * The file into which is accumulated the suggested commit message for
> > + * squash/fixup commands. When the first of a series of squash/fixups
>
> The same comment as 03/34 applies here, regarding blank line to
> separate logical unit.
Same rationale here: the path functions are one big continuous block, with
comments obviously applying to the immediately following line only.
> > +static int update_squash_messages(enum todo_command command,
> > + struct commit *commit, struct replay_opts *opts)
> > +{
> > + struct strbuf buf = STRBUF_INIT;
> > + int count, res;
> > + const char *message, *body;
> > +
> > + if (file_exists(rebase_path_squash_msg())) {
> > + char *p, *p2;
> > +
> > + if (strbuf_read_file(&buf, rebase_path_squash_msg(), 2048) <= 0)
> > + return error(_("could not read '%s'"),
> > + rebase_path_squash_msg());
> > +
> > + if (buf.buf[0] != comment_line_char ||
> > + !skip_prefix(buf.buf + 1, " This is a combination of ",
> > + (const char **)&p))
> > + return error(_("unexpected 1st line of squash message:"
> > + "\n\n\t%.*s"),
> > + (int)(strchrnul(buf.buf, '\n') - buf.buf),
> > + buf.buf);
> > + count = strtol(p, &p2, 10);
> > +
> > + if (count < 1 || *p2 != ' ')
> > + return error(_("invalid 1st line of squash message:\n"
> > + "\n\t%.*s"),
> > + (int)(strchrnul(buf.buf, '\n') - buf.buf),
> > + buf.buf);
> > +
> > + sprintf((char *)p, "%d", ++count);
>
> Do we know the area pointed at p (which is inside buf) long enough
> not to overflow?
Yes, we know that: p2 points to the byte after the parsed number, and said
byte is a space (ASCII 0x20), as verified by the if() above.
> If the original were 9 and you incremented to get 10, you would need one
> extra byte.
Exactly. That extra byte (if needed) is 0x20, as verified above, and can
be overwritten.
If that extra byte (to which p2 points) is *not* overwritten, i.e. if the
new count requires the same amount of space in decimal representation as
the previous count, it is now NUL, as tested here:
> > + if (!*p2)
> > + *p2 = ' ';
> > + else {
> > + *(++p2) = 'c';
>
> p2 points into buf; do we know this increment does not step beyond
> its end? What is the meaning of a letter 'c' here (I do not see a
> corresponding one in the scripted update_squash_messages)?
This clause is entered only when the space needed by the previous count
was not sufficient to hold the new count, at which point we know that not
only the space after the old count was overwritten, but also the 'c' of
the "commits" string.
Therefore, this clause reinstates the 'c' and inserts the space, so that
everything is groovy again:
> > + strbuf_insert(&buf, p2 - buf.buf, " ", 1);
> > + }
Having said that, I just realized that this code was only safe as long as
the squash messages were not localized.
I changed the code to imitate more closely what the shell script does. It
made the code a little more verbose, but it should work better as a
consequence, and I am pretty certain you will find it easier to verify
that it is correct.
> > + }
> > + else {
>
> Style: "} else {" (I won't repeat this, as it will become too noisy).
It is not necessary to repeat this, either, as I took the first such
comment as a strong hint to look at the entire patch series and fix it
appropriately.
>
> > + unsigned char head[20];
> > + struct commit *head_commit;
> > + const char *head_message, *body;
> > +
> > + if (get_sha1("HEAD", head))
> > + return error(_("need a HEAD to fixup"));
> > + if (!(head_commit = lookup_commit_reference(head)))
> > + return error(_("could not read HEAD"));
> > + if (!(head_message = get_commit_buffer(head_commit, NULL)))
> > + return error(_("could not read HEAD's commit message"));
> > +
> > + body = strstr(head_message, "\n\n");
> > + if (!body)
> > + body = "";
> > + else
> > + body = skip_blank_lines(body + 2);
>
> I think I saw you used a helper function find_commit_subject() to do
> the above in an earlier patch for "edit" in this series. Would it
> make this part (and another one for "commit" we have after this
> if/else) shorter?
Right, and by using the helper function, I also fixed a bug handling funny
commit objects that had more than one empty line separating header from
the message.
Of course, there is a third location in sequencer.c (predating my patches)
that uses the very same idiom. Yet another patch added to this growing
patch series...
> > static int do_pick_commit(enum todo_command command, struct commit *commit,
> > - struct replay_opts *opts)
> > + struct replay_opts *opts, int final_fixup)
> > {
> > + int edit = opts->edit, cleanup_commit_message = 0;
> > + const char *msg_file = edit ? NULL : git_path_merge_msg();
> > unsigned char head[20];
> > struct commit *base, *next, *parent;
> > const char *base_label, *next_label;
> > struct commit_message msg = { NULL, NULL, NULL, NULL };
> > struct strbuf msgbuf = STRBUF_INIT;
> > - int res, unborn = 0, allow;
> > + int res, unborn = 0, amend = 0, allow;
> >
> > if (opts->no_commit) {
> > /*
> > @@ -749,7 +885,7 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,
> > else
> > parent = commit->parents->item;
> >
> > - if (opts->allow_ff &&
> > + if (opts->allow_ff && !is_fixup(command) &&
> > ((parent && !hashcmp(parent->object.oid.hash, head)) ||
> > (!parent && unborn)))
> > return fast_forward_to(commit->object.oid.hash, head, unborn, opts);
> > @@ -813,6 +949,28 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,
> > }
> > }
> >
> > + if (is_fixup(command)) {
> > + if (update_squash_messages(command, commit, opts))
> > + return -1;
> > + amend = 1;
> > + if (!final_fixup)
> > + msg_file = rebase_path_squash_msg();
> > + else if (file_exists(rebase_path_fixup_msg())) {
> > + cleanup_commit_message = 1;
> > + msg_file = rebase_path_fixup_msg();
> > + }
> > + else {
> > + const char *dest = git_path("SQUASH_MSG");
> > + unlink(dest);
> > + if (copy_file(dest, rebase_path_squash_msg(), 0666))
> > + return error(_("could not rename '%s' to '%s'"),
> > + rebase_path_squash_msg(), dest);
>
> Perhaps an error from unlink(dest) before copy_file() should also
> result in an error return?
No, because the most likely cause for that `unlink()` to fail is that the
destination does not exist, and it is fine if it does not exist yet.
No worries, copy_file() will fail if we could not remove any existing file
and we still error out in that case.
> > + unlink(git_path("MERGE_MSG"));
>
> Errors from this and other unlink() that emulates "rm -f" were
> unchecked in the scripted original, so not checking for errors is
> not a regression. I would check for an error if I were writing
> this, however, because I know I would forget updating these after I
> am done with the series.
The problem is that these files do not necessarily exist. We only unlink()
them to make sure that they do not exist afterwards.
In any case, I am still more interested in a faithful translation than
already starting to improve the code at this stage.
> > @@ -1475,6 +1660,21 @@ static int do_exec(const char *command_line)
> > return status;
> > }
> >
> > +static int is_final_fixup(struct todo_list *todo_list)
> > +{
> > + int i = todo_list->current;
> > +
> > + if (!is_fixup(todo_list->items[i].command))
> > + return 0;
> > +
> > + while (++i < todo_list->nr)
> > + if (is_fixup(todo_list->items[i].command))
> > + return 0;
> > + else if (todo_list->items[i].command < TODO_NOOP)
> > + break;
>
> What follows NOOP are comment and "drop" which is another comment in
> disguise, so this one is excluding all the no-op commands in various
> shapes, which makes sense but is clear only to a reader who bothered
> to go back to "enum todo_command" and checked that fact. If a check
> for "is it one of the no-op commands?" appears only here, a single
> liner comment may be sufficient (but necessary) to help readers.
> Otherwise a single-liner helper function (similar to is_fixup() you
> have) with a descriptive name would be better than a single liner
> comment.
True. I introduced is_noop().
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH v2 07/34] sequencer (rebase -i): add support for the 'fixup' and 'squash' commands
From: Johannes Schindelin @ 2016-12-19 16:59 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git, Kevin Daudt, Dennis Kaarsemaker
In-Reply-To: <20161215185630.xxeimack63wqwv2e@sigill.intra.peff.net>
Hi Peff,
On Thu, 15 Dec 2016, Jeff King wrote:
> On Thu, Dec 15, 2016 at 10:42:53AM -0800, Junio C Hamano wrote:
>
> > > + sprintf((char *)p, "%d", ++count);
> >
> > Do we know the area pointed at p (which is inside buf) long enough
> > not to overflow? If the original were 9 and you incremented to get
> > 10, you would need one extra byte.
>
> Even if it is enough, I'd ask to please use xsnprintf(). In the off
> chance that there's a programming error, we'd get a nice die("BUG")
> instead of a buffer overflow (and it makes the code base easier to audit
> for other overflows).
I ended up with more verbose, easier-to-read code that does not try to do
things in-place, in favor of being slightly more wasteful with strbufs.
Ciao,
Dscho
^ permalink raw reply
* [PATCH] Tweak help auto-correct phrasing.
From: Marc Branchaud @ 2016-12-19 17:01 UTC (permalink / raw)
To: git; +Cc: Kaartic Sivaraam, Chris Packham, Alex Riesen
In-Reply-To: <CAFOYHZDnpzdYq9j4-xGSdKZQX9deLBpZZhz209qV7cCtq537SA@mail.gmail.com>
Signed-off-by: Marc Branchaud <marcnarc@xiplink.com>
---
On 2016-12-18 07:48 PM, Chris Packham wrote:
>
> This feature already exists (although it's not interactive). See
> help.autoCorrect in the git-config man page. "git config
> help.autoCorrect -1" should to the trick.
Awesome, I was unaware of this feature. Thanks!
I found the message it prints a bit awkward, so here's a patch to fix it up.
Instead of:
WARNING: You called a Git command named 'lgo', which does not exist.
Continuing under the assumption that you meant 'log'
in 1.5 seconds automatically...
it's now:
WARNING: You called a Git command named 'lgo', which does not exist.
Continuing in 1.5 seconds under the assumption that you meant 'log'.
M.
help.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/help.c b/help.c
index 53e2a67e00..55350c0673 100644
--- a/help.c
+++ b/help.c
@@ -381,12 +381,18 @@ const char *help_unknown_cmd(const char *cmd)
clean_cmdnames(&main_cmds);
fprintf_ln(stderr,
_("WARNING: You called a Git command named '%s', "
- "which does not exist.\n"
- "Continuing under the assumption that you meant '%s'"),
- cmd, assumed);
- if (autocorrect > 0) {
- fprintf_ln(stderr, _("in %0.1f seconds automatically..."),
- (float)autocorrect/10.0);
+ "which does not exist."),
+ cmd);
+ if (autocorrect < 0)
+ fprintf_ln(stderr,
+ _("Continuing under the assumption that "
+ "you meant '%s'."),
+ assumed);
+ else {
+ fprintf_ln(stderr,
+ _("Continuing in %0.1f seconds under the "
+ "assumption that you meant '%s'."),
+ (float)autocorrect/10.0, assumed);
sleep_millisec(autocorrect * 100);
}
return assumed;
--
2.11.0.dirty
^ permalink raw reply related
* Re: [PATCH v2 09/34] sequencer (rebase -i): write an author-script file
From: Johannes Schindelin @ 2016-12-19 17:06 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Kevin Daudt, Dennis Kaarsemaker
In-Reply-To: <xmqqd1gtuivc.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Thu, 15 Dec 2016, Junio C Hamano wrote:
> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
>
> > + strbuf_addstr(&buf, "GIT_AUTHOR_NAME='");
> > + while (*message && *message != '\n' && *message != '\r')
> > + if (skip_prefix(message, " <", &message))
> > + break;
> > + else if (*message != '\'')
> > + strbuf_addch(&buf, *(message++));
> > + else
> > + strbuf_addf(&buf, "'\\\\%c'", *(message++));
> > + strbuf_addstr(&buf, "'\nGIT_AUTHOR_EMAIL='");
> > + while (*message && *message != '\n' && *message != '\r')
> > + if (skip_prefix(message, "> ", &message))
> > + break;
> > + else if (*message != '\'')
> > + strbuf_addch(&buf, *(message++));
> > + else
> > + strbuf_addf(&buf, "'\\\\%c'", *(message++));
>
> Aren't these reading from an in-core commit object?
>
> If so, it should use split_ident_line() for consistency with other
> parts of the system to do this parsing. We should also already have
> a helper for simple shell-quoting in quote.c and you would want to
> use that instead of open coding like this.
We keep coming back to the same argument. You want this quoting/dequoting
to be turned into a full-fledged parser. And I keep pointing out that the
code here does not *need* to parse but only construct an environment
block.
Hopefully the next iteration that integrates Peff's suggestions will find
more of your approval.
Ciao,
Dscho
^ permalink raw reply
* Re: Bug report: $program_name in error message
From: Stefan Beller @ 2016-12-19 17:07 UTC (permalink / raw)
To: Josh Bleecher Snyder, vascomalmeida; +Cc: git@vger.kernel.org
In-Reply-To: <CAFAcib9-rUSqyBRpauw3pTf9OPTKLYNf7bdh2gyykBNtJTZKGg@mail.gmail.com>
+ Vasco Almeida, who authored d323c6b641,
(i18n: git-sh-setup.sh: mark strings for translation, 2016-06-17)
On Sun, Dec 18, 2016 at 1:44 PM, Josh Bleecher Snyder
<josharian@gmail.com> wrote:
>>> To reproduce, run 'git submodule' from within a bare repo. Result:
>>>
>>> $ git submodule
>>> fatal: $program_name cannot be used without a working tree.
>>>
>>> Looks like the intent was for $program_name to be interpolated.
>>
>> Which version of git do you use?
>
> $ git version
> git version 2.11.0
>
>
>>> As an aside, I sent a message a few days ago about a segfault when
>>> working with a filesystem with direct_io on, but it appears not to
>>> have made it to the archives on marc.info. Am I perhaps still
>>> greylisted?
>>
>> Both emails show up in my mailbox (subscribed to the mailing list),
>> so I think that just nobody answered your first email as the answer
>> may be non trivial.
>
> Thanks for the confirmation.
>
> -josh
^ 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