* Re: [PATCH] tests: grep portability fixes
From: Andreas Ericsson @ 2008-09-30 10:12 UTC (permalink / raw)
To: Jeff King; +Cc: Shawn O. Pearce, git
In-Reply-To: <20080930080355.GA19605@sigill.intra.peff.net>
Jeff King wrote:
> We try to avoid using the "-q" or "-e" options, as they are
> largely useless, as explained in aadbe44f.
>
> There is one exception for "-e" here, which is in t7701 used
> to produce an "or" of patterns. This can be rewritten as an
> egrep pattern.
>
> This patch also removes use of "grep -F" in favor of the
> more widely available "fgrep".
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>
> diff --git a/t/t0002-gitfile.sh b/t/t0002-gitfile.sh
> index 4db4ac4..cb14425 100755
> --- a/t/t0002-gitfile.sh
> +++ b/t/t0002-gitfile.sh
> @@ -32,7 +32,7 @@ test_expect_success 'bad setup: invalid .git file format' '
> echo "git rev-parse accepted an invalid .git file"
> false
> fi &&
> - if ! grep -qe "Invalid gitfile format" .err
> + if ! grep "Invalid gitfile format" .err
Doesn't output need to be redirected when you drop '-q'?
Other than that, this looks good after a quick scan.
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply
* Re: [PATCH] doc: enhance git describe --tags help
From: Andreas Ericsson @ 2008-09-30 10:09 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Shawn O. Pearce, Pierre Habouzit, Erez Zilber,
git@vger.kernel.org, open-iscsi, Junio C Hamano
In-Reply-To: <20080930095641.GA9001@strlen.de>
Uwe Kleine-König wrote:
> Hello,
>
> On Mon, Sep 29, 2008 at 08:01:27AM -0700, Shawn O. Pearce wrote:
>> --tags::
>> If a lightweight tag exactly matches, output it. If no
>> annotated tag is found in the ancestry but a lightweight
>> tag is found, output the lightweight tag.
> IMHO --tags should behave as Erez expected (because it's what I
> expected, too). As --tags currently behaves it's only usable in very
> rare cases (most of the time it only makes a difference on repos without
> any annotated tag).
>
> When do you pass --tags? Only if a lightweight tag is OK for an answer.
> And then I would prefer a "near" lightweight tag to a "farer" annotated
> one.
>
The reason lightweight tags aren't considered is that they're often put
somewhere to just mark some refs one wants to keep around, or as a sort
of movable snapshot marker (we have "latest/build", "latest/tested" etc
as lightweight tags). It's nifty to use lw tags for that since they can't
be committed to by accident. That doesn't mean we want 'git describe' to
start outputting "latest-build" whenever we want to pull a version number
from it though.
This was especially true before the creation of "git stash" and the reflog,
but quite a lot of people still use them for similar purposes.
In short; "git describe" output stays the way it is, or things will start
unexpectedly breaking for quite a lot of people.
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply
* Re: [PATCH] Builtin-commit: show on which branch a commit was added
From: Andreas Ericsson @ 2008-09-30 9:59 UTC (permalink / raw)
To: Jeff King
Cc: Wincent Colaiuta, Pieter de Bie, Junio C Hamano, Git Mailinglist
In-Reply-To: <20080930070938.GA14757@sigill.intra.peff.net>
Jeff King wrote:
> On Tue, Sep 30, 2008 at 08:37:00AM +0200, Wincent Colaiuta wrote:
>
>> As far as long-line-wrapping goes, I don't really think this is a problem
>> for Git to solve (by truncation or any other means); it's more of a user
>> behaviour thing where one would hope that users would get into the habit
>> of using concise subject lines and branch names.
>
> How concise must we be? I wrap my commit messages at 60 characters,
> which I consider quite conservative. But
>
> Created commit abcd1234 on jk/my-topic-branch:
>
> takes up over half of an 80-column terminal. Is that a long branch name?
> Browsing "git log --grep=Merge.branch --pretty=format:%s origin/next"
> suggests it's not terribly out of line (at least by Junio's standards).
>
> Dropping "commit " will help some. But given how much width is still
> used, and the fact that this message is really just to say "yes, I
> confirm that we just created the commit you asked for", I think
> truncating (with dots) to keep it within 80 characters is reasonable.
>
I agree. Obvious solution is to do
subj_len = term_width - (strlen(cruft) + strlen(branch_name))
where strlen(cruft) is just 8 less if we drop 'commit ' from the
cases. See the patch I just sent though. I sort of like that one.
Another way would be to write
<branch>: Created <hash>: "subject line..."
As <hash> will very, very rarely match anything the user would put
in his/her commit message themselves. Quoting the subject is probably
a nice touch, and it can make sense to put it last as it's the least
interesting of the things we print. Ah well. I'll just await commentary
on the patch I've already sent before I go ahead and do something like
that.
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply
* Re: [PATCH] doc: enhance git describe --tags help
From: Uwe Kleine-König @ 2008-09-30 9:56 UTC (permalink / raw)
To: Shawn O. Pearce
Cc: Pierre Habouzit, Erez Zilber, git@vger.kernel.org, open-iscsi,
Junio C Hamano
In-Reply-To: <20080929150127.GB18340@spearce.org>
Hello,
On Mon, Sep 29, 2008 at 08:01:27AM -0700, Shawn O. Pearce wrote:
> --tags::
> If a lightweight tag exactly matches, output it. If no
> annotated tag is found in the ancestry but a lightweight
> tag is found, output the lightweight tag.
IMHO --tags should behave as Erez expected (because it's what I
expected, too). As --tags currently behaves it's only usable in very
rare cases (most of the time it only makes a difference on repos without
any annotated tag).
When do you pass --tags? Only if a lightweight tag is OK for an answer.
And then I would prefer a "near" lightweight tag to a "farer" annotated
one.
Best regards
Uwe
^ permalink raw reply
* [PATCH] git commit: Reformat output somewhat
From: Andreas Ericsson @ 2008-09-30 9:52 UTC (permalink / raw)
To: Pieter de Bie, Jeff King, Git Mailing List, Shawn Pearce,
Junio C Hamano
In-Reply-To: <20080930061654.GA14584@sigill.intra.peff.net>
Previously, we used to print something along the lines of
Created commit abc9056 on master: Snib the sprock
but that output was sometimes confusing, as many projects use
the "subsystem: message" style of commit subjects (just like
this commit message does). When such improvements are done on
topic-branches, it's not uncommon to name the topic-branch the
same as the subsystem, leading to output like this:
Created commit abc9056 on i386: i386: Snib the sprock
which doesn't look very nice and can be highly confusing.
This patch alters the format so that the noise-word "commit"
is dropped except when it makes the output read better and
the commit subject is put inside parentheses. We also
emphasize the detached case so that users do not overlook it
in case the commit subject is long enough to extend to the
next line. The end result looks thusly:
normal case
Created abc9056 (i386: Snib the sprock) on i386
detached head
Created DETACHED commit abc9056 (i386: Snib the sprock)
While we're at it, we rename "initial commit" to "root-commit"
to align it with the argument to 'git log', producing this:
initial commit
Created root-commit abc9056 (i386: Snib the sprock) on i386
Documentation/gittutorial-2.txt is updated accordingly so that
new users recognize what they're looking at.
Signed-off-by: Andreas Ericsson <ae@op5.se>
---
According to the few entries in the discussion about showing the
branch we're on, this patch should probably go on top of next
fairly soon.
If the code-change isn't accepted, let me know and I'll fix the
documentation update to match whatever goes in builtin-commit.c.
Feel free to alter shouty-caps for detached when applying. I have
no strong opinion either way, as I never commit on detached head
anyway.
Thanks.
Documentation/gittutorial-2.txt | 13 ++++++++-----
builtin-commit.c | 12 +++++-------
2 files changed, 13 insertions(+), 12 deletions(-)
diff --git a/Documentation/gittutorial-2.txt b/Documentation/gittutorial-2.txt
index 6609046..8484e7a 100644
--- a/Documentation/gittutorial-2.txt
+++ b/Documentation/gittutorial-2.txt
@@ -32,22 +32,25 @@ Initialized empty Git repository in .git/
$ echo 'hello world' > file.txt
$ git add .
$ git commit -a -m "initial commit"
-Created initial commit 54196cc2703dc165cbd373a65a4dcf22d50ae7f7
+Created root-commit 54196cc (initial commit) on master
create mode 100644 file.txt
$ echo 'hello world!' >file.txt
$ git commit -a -m "add emphasis"
-Created commit c4d59f390b9cfd4318117afde11d601c1085f241
+Created c4d59f3 (add emphasis) on master
------------------------------------------------
-What are the 40 digits of hex that git responded to the commit with?
+What are the 7 digits of hex that git responded to the commit with?
We saw in part one of the tutorial that commits have names like this.
It turns out that every object in the git history is stored under
-such a 40-digit hex name. That name is the SHA1 hash of the object's
+a 40-digit hex name. That name is the SHA1 hash of the object's
contents; among other things, this ensures that git will never store
the same data twice (since identical data is given an identical SHA1
name), and that the contents of a git object will never change (since
-that would change the object's name as well).
+that would change the object's name as well). The 7 char hex strings
+here are simply the abbreviation of such 40 character long strings.
+Abbreviations can be used everywhere where the 40 character strings
+can be used, so long as they are unambiguous.
It is expected that the content of the commit object you created while
following the example above generates a different SHA1 hash than
diff --git a/builtin-commit.c b/builtin-commit.c
index 161128b..f0765cc 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -884,12 +884,11 @@ static char *get_commit_format_string(void)
const char *head = resolve_ref("HEAD", sha, 0, NULL);
struct strbuf buf = STRBUF_INIT;
- strbuf_addstr(&buf, "format:%h");
+ /* use shouty-caps if we're on detached HEAD */
+ strbuf_addf(&buf, "format:%s", strcmp("HEAD", head) ? "" : "DETACHED commit");
+ strbuf_addstr(&buf, "%h (%s)");
- /* Are we on a detached HEAD? */
- if (!strcmp("HEAD", head))
- strbuf_addstr(&buf, " on detached HEAD");
- else if (!prefixcmp(head, "refs/heads/")) {
+ if (!prefixcmp(head, "refs/heads/")) {
const char *cp;
strbuf_addstr(&buf, " on ");
for (cp = head + 11; *cp; cp++) {
@@ -899,7 +898,6 @@ static char *get_commit_format_string(void)
strbuf_addch(&buf, *cp);
}
}
- strbuf_addstr(&buf, ": %s");
return strbuf_detach(&buf, NULL);
}
@@ -933,7 +931,7 @@ static void print_summary(const char *prefix, const unsigned char *sha1)
rev.diffopt.break_opt = 0;
diff_setup_done(&rev.diffopt);
- printf("Created %scommit ", initial_commit ? "initial " : "");
+ printf("Created %s", initial_commit ? "root-commit " : "");
if (!log_tree_commit(&rev, commit)) {
struct strbuf buf = STRBUF_INIT;
--
1.6.0.2.529.g37dbc.dirty
^ permalink raw reply related
* Re: [PATCH] Builtin-commit: show on which branch a commit was added
From: Andreas Ericsson @ 2008-09-30 9:45 UTC (permalink / raw)
To: Jeff King; +Cc: Pieter de Bie, Junio C Hamano, Git Mailinglist
In-Reply-To: <20080930061654.GA14584@sigill.intra.peff.net>
Jeff King wrote:
> On Tue, Sep 30, 2008 at 08:13:51AM +0200, Andreas Ericsson wrote:
>
>> Created 6207abc (subwidget: one quite long line of sum...) on <branch>
>>
>> "commit" is just noise. Parentheses are often used to extemporize when
>> using normal written language so it should work well here too.
>
> I do like that better, and there is some precedent in the way we mention
> commits in emails (I know Junio even has an alias that formats it as
> $hash ($subject)).
>
> By your "..." are you suggesting to truncate the subject?
>
Yes, although that can be added later. I'm sending a format fix to the
list in a minute or two.
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply
* Re: [PATCH 1/6] gitweb: action in path with use_pathinfo
From: Jakub Narebski @ 2008-09-30 8:48 UTC (permalink / raw)
To: Giuseppe Bilotta; +Cc: git, Petr Baudis, Lea Wiemann
In-Reply-To: <cb7bb73a0809300105s24706d79hb40e147739ec6f05@mail.gmail.com>
On Tue, 30 Sep 2008, Giuseppe Bilotta wrote:
> On Tue, Sep 30, 2008 at 2:21 AM, Jakub Narebski <jnareb@gmail.com> wrote:
[...]
>> Ah. Now I understand.
>>
>> When creating code for href(..., -replay=>1), which by the way I thought
>> would be more useful than actually is, I have forgot that parameters to
>> gitweb could be passed in other way that through CGI parameters
>> (CGI query)[1].
>>
>> Using
>>
>> $params{$name} = [ $cgi->param($symbol) ];
>>
>> is a cute hack, but it doesn't work for arguments passed via path_info
>> (was: project, hash_base and file_name; while now it is project, action,
>> hash_base (in full) and file_name).
[...]
>> The solution I thought about and abandoned in favor of this cute hack
>> was to have additional hash (in addition to %mapping), which would map
>> action names to references to variables holding the value for parameter.
[...]
>> I am talking there about the following solution:
>>
>> my %action_vars = (
>> project => \$project,
>> action => \$action,
>> # ...
>> extra_options => \@extra_options,
>> );
>> # ...
>> while (my ($name, $symbol) = each %mapping) {
>> if (!exists $params{$name}) {
>> $params{$name} = ${$action_vars{$name}};
>> }
>> }
>>
>>
>> This avoids cure hack of (from your code)
>>
>> } else {
>> no strict 'refs';
>> $params{$name} = $$name if $$name;
>> }
>>
>> I think that gitweb should use single source, not CGI query parameters
>> or variable saving [sanitized] value.
>
> The alternative I've been thinking about would be to have an
> %input_parameters hash that holds all input parameters regardless of
> hash; thus CGI query parameters and data extracted from PATH_INFO,
> presently, but also command line options in the future, or whatever
> else.
>
> This is somewhat different from your %action_vars alternative, in the
> sense that it isolates _input_ data, whereas if I understand correctly
> the approach you suggest would isolate _output_ data (in the sense of
> data to be used during link creation and whatnot).
>
> Presently, the gitweb code defines some $variables from the input
> parameters, and then overwrites them for output. Keeping the input
> stuff clearly separate from the output stuff would mean that any
> routine can retrieve the input data regardless of the subsequent
> mangling and without any need to make ad-hoc backups or other tricks.
>
> So my proposal is that I implement this %input_params stuff as the
> first patch for the pathinfo series, and use %input_params all around
> where cgi parameters are used currently (of course, %input_params is
> initialized with the CGI parameters at first). The next patch would be
> the extraction of parameters from PATH_INFO. And thirdly the PATH_INFO
> URL generation (with or without the /-before-filename thing, at your
> preference)
I presume that you would want to replace for example $hash_base
everywhere by %input_params{'hash_base'}?
I can think of yet another solution, namely to abstract getting
parameters from CGI query string, from path_info, and possibly in the
future also from command line options, and use this mechanism in
the getting parameters and validation part.
The %params hash would be filled from CGI parameters by using simply
"%params = $cgi->Vars;", then added to in evaluate_path_info instead
of directly modifying global parameters variables. The input validation
and dispatch part would be modified to use %params (taking care of
multivalued parameters as described in CGI(3pm)), like below:
our $action = $params{'a'} || $params{'action'};
if (defined $action) {
if ($action =~ m/[^0-9a-zA-Z\.\-_]/) {
die_error(400, "Invalid action parameter");
}
}
That is just for consideration: each approach has its advantages and
disadvantages. Your proposal, as I understand it, is similar to the
way described in "Storing options in a hash" subsection of
Getopt::Long(3pm) manpage.
Or we could just scrap and revert adding href(..., -replay=>1).
There is much trouble with getting it right and performing well,
and it is less useful than I thought (at least now).
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [PATCH] parse-opt: migrate fmt-merge-msg.
From: Pierre Habouzit @ 2008-09-30 8:40 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git, gitster
In-Reply-To: <20080929163523.GC18340@spearce.org>
[-- Attachment #1: Type: text/plain, Size: 1012 bytes --]
On Mon, Sep 29, 2008 at 04:35:23PM +0000, Shawn O. Pearce wrote:
> Pierre Habouzit <madcoder@debian.org> wrote:
> > Also fix an inefficient printf("%s", ...) where we can use write_in_full.
> >
> > Signed-off-by: Pierre Habouzit <madcoder@debian.org>
> > ---
> > builtin-fmt-merge-msg.c | 50 +++++++++++++++++++++-------------------------
> > 1 files changed, 23 insertions(+), 27 deletions(-)
>
> Near as I can tell, this is based upon a merge commit in next.
>
> We can't do that. Patches need to be based on master, or worst-case
> on a topic head that is in next or pu (in which case the name of
> the topic, or its tip commit, is helpful in the note).
Hmm I've always sent my patches this way, and I believe you can git am
-3 them on top of master easily. I can send you the updated series if
you want.
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: [PATCH] doc: enhance git describe --tags help
From: Pierre Habouzit @ 2008-09-30 8:39 UTC (permalink / raw)
To: Shawn O. Pearce
Cc: Erez Zilber, git@vger.kernel.org, open-iscsi, Junio C Hamano
In-Reply-To: <20080929150127.GB18340@spearce.org>
[-- Attachment #1: Type: text/plain, Size: 1293 bytes --]
On Mon, Sep 29, 2008 at 03:01:27PM +0000, Shawn O. Pearce wrote:
> Pierre Habouzit <madcoder@debian.org> wrote:
> > diff --git a/Documentation/git-describe.txt b/Documentation/git-describe.txt
> > index c4dbc2a..9cc8c2f 100644
> > --- a/Documentation/git-describe.txt
> > +++ b/Documentation/git-describe.txt
> > @@ -30,7 +30,8 @@ OPTIONS
> >
> > --tags::
> > Instead of using only the annotated tags, use any tag
> > - found in `.git/refs/tags`.
> > + found in `.git/refs/tags`. Though if an annotated tag is found in the
> > + ancestry, it will always be preferred to lightweight tags.
>
> As technically correct as the statement is, I read this and go
> "why do we even have --tags?".
>
> If I read builtin-describe.c right we only honor --tags on an exact
> match, or if there are no annotated tags at all in the history.
> I wonder if docs like this aren't better for --tags:
>
> --tags::
> If a lightweight tag exactly matches, output it. If no
> annotated tag is found in the ancestry but a lightweight
> tag is found, output the lightweight tag.
sounds better indeed.
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: [PATCH] remove vim syntax highlighting in favor of upstream
From: Michael J Gruber @ 2008-09-30 8:29 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: Shawn O. Pearce, Jeff King, vim, git
In-Reply-To: <20080929200814.GA19840@neumann>
SZEDER Gábor venit, vidit, dixit 29.09.2008 22:08:
> As of version 7.2, vim ships with its own syntax
> highlighting for git commit messages, which is:
>
> 1. more comprehensive in splitting up the various
> components of the file
>
> 2. in accordance with the usual vim behavior for syntax
> highlighting (e.g., respecting b:current_syntax)
>
> 3. presumably better maintained (I have not been using
> what's in git's contrib/ directory for some time in
> favor of the upstream version)
>
> Furthermore, vim upsream also provides syntax highlighting
> for other git filetypes (gitconfig, rebase, send-email).
>
> This patch gets rid of our local version and just points
> interested parties to the upstream version.
>
> The code for auto-detecting filetypes is taken from vim's
> runtime/filetype.vim.
>
> Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
> ---
>
> On Mon, Sep 29, 2008 at 07:55:42AM -0700, Shawn O. Pearce wrote:
>> Missing SBO line?
> Here it is. Since significant parts of the patch and the commit
> message are from Jeff, maybe he should sign off, too?
>
> Note, that this patch is slightly different from the previous one, as
> it proposes writing the auto-detect commands into ~/.vim/filetype.vim
> instead of ~/.vimrc. It's not quite clear to me why, but it seems to
> resolve the filetype confusion I mentioned in my previous email.
>
>
> contrib/vim/README | 38 ++++++++++++++++++++++++++++++--------
> contrib/vim/syntax/gitcommit.vim | 18 ------------------
> 2 files changed, 30 insertions(+), 26 deletions(-)
> delete mode 100644 contrib/vim/syntax/gitcommit.vim
>
> diff --git a/contrib/vim/README b/contrib/vim/README
> index 9e7881f..c487346 100644
> --- a/contrib/vim/README
> +++ b/contrib/vim/README
> @@ -1,8 +1,30 @@
> -To syntax highlight git's commit messages, you need to:
> - 1. Copy syntax/gitcommit.vim to vim's syntax directory:
> - $ mkdir -p $HOME/.vim/syntax
> - $ cp syntax/gitcommit.vim $HOME/.vim/syntax
> - 2. Auto-detect the editing of git commit files:
> - $ cat >>$HOME/.vimrc <<'EOF'
> - autocmd BufNewFile,BufRead COMMIT_EDITMSG set filetype=gitcommit
> - EOF
> +Syntax highlighting for git commit messages, config files, etc. is
> +included with the vim distribution as of vim 7.2, and should work
> +automatically.
> +
> +If you have an older version of vim, you can get the latest syntax
> +files from the vim project:
> +
> + http://vim.svn.sourceforge.net/viewvc/vim/trunk/runtime/syntax/git.vim
> + http://vim.svn.sourceforge.net/viewvc/vim/trunk/runtime/syntax/gitcommit.vim
> + http://vim.svn.sourceforge.net/viewvc/vim/trunk/runtime/syntax/gitconfig.vim
> + http://vim.svn.sourceforge.net/viewvc/vim/trunk/runtime/syntax/gitrebase.vim
> + http://vim.svn.sourceforge.net/viewvc/vim/trunk/runtime/syntax/gitsendemail.vim
> +
> +To install:
> +
> + 1. Copy these files to vim's syntax directory $HOME/.vim/syntax
> + 2. To auto-detect the editing of various git-related filetypes:
> + $ cat >>$HOME/.vim/filetype.vim <<'EOF'
> + autocmd BufNewFile,BufRead *.git/COMMIT_EDITMSG setf gitcommit
> + autocmd BufNewFile,BufRead *.git/config,.gitconfig setf gitconfig
> + autocmd BufNewFile,BufRead git-rebase-todo setf gitrebase
> + autocmd BufNewFile,BufRead .msg.[0-9]*
> + \ if getline(1) =~ '^From.*# This line is ignored.$' |
> + \ setf gitsendemail |
> + \ endif
> + autocmd BufNewFile,BufRead *.git/**
> + \ if getline(1) =~ '^\x\{40\}\>\|^ref: ' |
> + \ setf git |
> + \ endif
> + EOF
Works as described with vim 7.1.
How about creating a syntax file for editing the files generated by
format-patch, especially 0000-cover-letter.patch? Should be mostly a
combination of "git.vim" and "gitsendemail.vim", but I didn't find any
syntax defs for the diffstat.
Michael
P.S.: What? Git doc linking to an svn repo? Someone needs to set up a
git mirror ;)
^ permalink raw reply
* [PATCH (GIT-GUI)] git-gui: Make Ctrl-T safe to use for conflicting files.
From: Alexander Gavrilov @ 2008-09-30 8:12 UTC (permalink / raw)
To: Git Mailing List; +Cc: Shawn O. Pearce, Johannes Sixt
A previous patch added a check for conflict markers, which
is done when the file is about to be staged due to a click
on the icon. However, pressing Ctrl-T still immediately
stages the file without confirmation. This patch fixes it.
The check requires a loaded diff, so staging multiple files
at once won't work if they are unmerged.
Signed-off-by: Alexander Gavrilov <angavrilov@gmail.com>
---
git-gui.sh | 4 +++-
lib/index.tcl | 11 +++++++++--
lib/merge.tcl | 1 +
lib/mergetool.tcl | 7 ++++---
4 files changed, 17 insertions(+), 6 deletions(-)
diff --git a/git-gui.sh b/git-gui.sh
index 4085e8f..717817e 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -2090,7 +2090,9 @@ proc toggle_or_diff {w x y} {
if {$col == 0 && $y > 1} {
# Conflicts need special handling
if {[string first {U} $state] >= 0} {
- merge_stage_workdir $path $w $lno
+ # $w must always be $ui_workdir, but...
+ if {$w ne $ui_workdir} { set lno {} }
+ merge_stage_workdir $path $lno
return
}
diff --git a/lib/index.tcl b/lib/index.tcl
index b045219..d33896a 100644
--- a/lib/index.tcl
+++ b/lib/index.tcl
@@ -298,11 +298,18 @@ proc add_helper {txt paths} {
set after {}
foreach path $paths {
switch -glob -- [lindex $file_states($path) 0] {
+ _U -
+ U? {
+ if {$path eq $current_diff_path} {
+ unlock_index
+ merge_stage_workdir $path
+ return
+ }
+ }
_O -
?M -
?D -
- ?T -
- U? {
+ ?T {
lappend pathList $path
if {$path eq $current_diff_path} {
set after {reshow_diff;}
diff --git a/lib/merge.tcl b/lib/merge.tcl
index 5c01875..ac4c7de 100644
--- a/lib/merge.tcl
+++ b/lib/merge.tcl
@@ -40,6 +40,7 @@ The rescan will be automatically started now.
_O {
continue; # and pray it works!
}
+ _U
U? {
error_popup [mc "You are in the middle of a conflicted merge.
diff --git a/lib/mergetool.tcl b/lib/mergetool.tcl
index 6ab5701..1984c64 100644
--- a/lib/mergetool.tcl
+++ b/lib/mergetool.tcl
@@ -23,13 +23,14 @@ This operation can be undone only by restarting the merge." \
}
}
-proc merge_stage_workdir {path w lno} {
+proc merge_stage_workdir {path {lno {}}} {
global current_diff_path diff_active
+ global current_diff_side ui_workdir
if {$diff_active} return
- if {$path ne $current_diff_path} {
- show_diff $path $w $lno {} [list do_merge_stage_workdir $path]
+ if {$path ne $current_diff_path || $ui_workdir ne $current_diff_side} {
+ show_diff $path $ui_workdir $lno {} [list do_merge_stage_workdir $path]
} else {
do_merge_stage_workdir $path
}
--
1.6.0.20.g6148bc
^ permalink raw reply related
* Re: [PATCH 1/6] gitweb: action in path with use_pathinfo
From: Giuseppe Bilotta @ 2008-09-30 8:05 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git, Petr Baudis, Lea Wiemann
In-Reply-To: <200809300221.25094.jnareb@gmail.com>
On Tue, Sep 30, 2008 at 2:21 AM, Jakub Narebski <jnareb@gmail.com> wrote:
> Perhaps not in so large and detailed form... I guess explanation of
> using ':/' as separator should be put there as well, if you plan to
> squash those patches.
I will add the explanation for the :/ separator in the patch for URL
generation (forgot it in v3)
>>>> @@ -624,8 +636,13 @@ sub href (%) {
>>>> if ($params{-replay}) {
>>>> while (my ($name, $symbol) = each %mapping) {
>>>> if (!exists $params{$name}) {
>>>> - # to allow for multivalued params we use arrayref form
>>>> - $params{$name} = [ $cgi->param($symbol) ];
>>>> + if ($cgi->param($symbol)) {
>>>> + # to allow for multivalued params we use arrayref form
>>>> + $params{$name} = [ $cgi->param($symbol) ];
>>>> + } else {
>>>> + no strict 'refs';
>>>> + $params{$name} = $$name if $$name;
>>>> + }
>>>> }
>>>> }
>>>> }
>>>
>>> What this change is about? And why this change is _here_, in this
>>> commit? It is I think unrelated, and wrong change.
>>
>> This is about being able to recycle CGI parameters that came through
>> as part of path_info instead of the CGI parameter list. It might not
>> be the best way to recover it, though. I *did* have a few thoughts
>> about an alternative way that consisted of build a parameter list
>> merging CGI and path-info parameter, but since this approach seemed to
>> work, I went with it.
>
> Fact, I have totally forgot about this.
Me too actually (the first version of this patch is almost a year old,
and since I forgot to write down why I did a few things ... taught me
that I should start putting more comments, though 8-D)
>>> href(..., -replay=>1) is all about reusing current URL, perhaps with
>>> a few parameters changed, like for example pagination links differ only
>>> in page number param change. For example if we had only hb= and f=
>>> parameters, -replay=>1 links should use only those, and not add h=
>>> parameter because somewhere we felt that we need $hash to be calculated.
>>
>> Assume for example that you are to an url such as
>>
>> http://git.oblomov.eu/git/tree/refs/remotes/origin/master:gitweb
>>
>> Without this patch, the 'history' link on the second header line links
>> to ARRAY(0xblah)ARRAY(0xblah). With this patch, it shows the proper
>> link. So either replay is being abused somewhere in the link
>> generation code, or this CGI+path_info parameter retrieval is
>> necessary, one way or the other.
>
> Ah. Now I understand.
>
> When creating code for href(..., -replay=>1), which by the way I thought
> would be more useful than actually is, I have forgot that parameters to
> gitweb could be passed in other way that through CGI parameters
> (CGI query)[1].
>
> Using
>
> $params{$name} = [ $cgi->param($symbol) ];
>
> is a cute hack, but it doesn't work for arguments passed via path_info
> (was: project, hash_base and file_name; while now it is project, action,
> hash_base (in full) and file_name).
Exactly.
> The solution I thought about and abandoned in favor of this cute hack
> was to have additional hash (in addition to %mapping), which would map
> action names to references to variables holding the value for parameter.
>
> This has the same problem as your proposed solution of putting some
> parameters which didn't come from URL but were filled from other info.
> $hash parameter is most likely to be culprit here.
>
> On the other hand it is more generic and doesn't rely on knowledge that
> there is no multi-valued parameter which can be expressed in path_info
> (currently only 'opt' parameter can be multi-valued, and requires
> arrayref, but also 'opt' parameter is and cannot be put in path_info).
>
> I am talking there about the following solution:
>
> my %action_vars = (
> project => \$project,
> action => \$action,
> # ...
> extra_options => \@extra_options,
> );
> # ...
> while (my ($name, $symbol) = each %mapping) {
> if (!exists $params{$name}) {
> $params{$name} = ${$action_vars{$name}};
> }
> }
>
>
> This avoids cure hack of (from your code)
>
> } else {
> no strict 'refs';
> $params{$name} = $$name if $$name;
> }
>
> I think that gitweb should use single source, not CGI query parameters
> or variable saving [sanitized] value.
The alternative I've been thinking about would be to have an
%input_parameters hash that holds all input parameters regardless of
hash; thus CGI query parameters and data extracted from PATH_INFO,
presently, but also command line options in the future, or whatever
else.
This is somewhat different from your %action_vars alternative, in the
sense that it isolates _input_ data, whereas if I understand correctly
the approach you suggest would isolate _output_ data (in the sense of
data to be used during link creation and whatnot).
Presently, the gitweb code defines some $variables from the input
parameters, and then overwrites them for output. Keeping the input
stuff clearly separate from the output stuff would mean that any
routine can retrieve the input data regardless of the subsequent
mangling and without any need to make ad-hoc backups or other tricks.
So my proposal is that I implement this %input_params stuff as the
first patch for the pathinfo series, and use %input_params all around
where cgi parameters are used currently (of course, %input_params is
initialized with the CGI parameters at first). The next patch would be
the extraction of parameters from PATH_INFO. And thirdly the PATH_INFO
URL generation (with or without the /-before-filename thing, at your
preference)
--
Giuseppe "Oblomov" Bilotta
^ permalink raw reply
* [PATCH] tests: grep portability fixes
From: Jeff King @ 2008-09-30 8:03 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git
We try to avoid using the "-q" or "-e" options, as they are
largely useless, as explained in aadbe44f.
There is one exception for "-e" here, which is in t7701 used
to produce an "or" of patterns. This can be rewritten as an
egrep pattern.
This patch also removes use of "grep -F" in favor of the
more widely available "fgrep".
Signed-off-by: Jeff King <peff@peff.net>
---
These are fallouts from getting (most of) the tests to pass
on Solaris.
t/t0002-gitfile.sh | 4 ++--
t/t1501-worktree.sh | 2 +-
t/t3700-add.sh | 2 +-
t/t4150-am.sh | 2 +-
t/t6040-tracking-info.sh | 4 ++--
t/t7002-grep.sh | 2 +-
t/t7701-repack-unpack-unreachable.sh | 4 ++--
t/t9500-gitweb-standalone-no-errors.sh | 2 +-
8 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/t/t0002-gitfile.sh b/t/t0002-gitfile.sh
index 4db4ac4..cb14425 100755
--- a/t/t0002-gitfile.sh
+++ b/t/t0002-gitfile.sh
@@ -32,7 +32,7 @@ test_expect_success 'bad setup: invalid .git file format' '
echo "git rev-parse accepted an invalid .git file"
false
fi &&
- if ! grep -qe "Invalid gitfile format" .err
+ if ! grep "Invalid gitfile format" .err
then
echo "git rev-parse returned wrong error"
false
@@ -46,7 +46,7 @@ test_expect_success 'bad setup: invalid .git file path' '
echo "git rev-parse accepted an invalid .git file path"
false
fi &&
- if ! grep -qe "Not a git repository" .err
+ if ! grep "Not a git repository" .err
then
echo "git rev-parse returned wrong error"
false
diff --git a/t/t1501-worktree.sh b/t/t1501-worktree.sh
index c039ee3..f6a6f83 100755
--- a/t/t1501-worktree.sh
+++ b/t/t1501-worktree.sh
@@ -171,7 +171,7 @@ test_expect_success 'git diff' '
test_expect_success 'git grep' '
(cd repo.git/work/sub &&
- GIT_DIR=../.. GIT_WORK_TREE=.. git grep -l changed | grep -q dir/tracked)
+ GIT_DIR=../.. GIT_WORK_TREE=.. git grep -l changed | grep dir/tracked)
'
test_done
diff --git a/t/t3700-add.sh b/t/t3700-add.sh
index 2ac93a3..9f6454d 100755
--- a/t/t3700-add.sh
+++ b/t/t3700-add.sh
@@ -226,7 +226,7 @@ test_expect_success 'git add '\''fo\[ou\]bar'\'' ignores foobar' '
git reset --hard &&
touch fo\[ou\]bar foobar &&
git add '\''fo\[ou\]bar'\'' &&
- git ls-files fo\[ou\]bar | grep -F fo\[ou\]bar &&
+ git ls-files fo\[ou\]bar | fgrep fo\[ou\]bar &&
! ( git ls-files foobar | grep foobar )
'
diff --git a/t/t4150-am.sh b/t/t4150-am.sh
index 1be5fb3..796f795 100755
--- a/t/t4150-am.sh
+++ b/t/t4150-am.sh
@@ -165,7 +165,7 @@ test_expect_success 'am --keep really keeps the subject' '
git am --keep patch4 &&
! test -d .git/rebase-apply &&
git cat-file commit HEAD |
- grep -q -F "Re: Re: Re: [PATCH 1/5 v2] third"
+ fgrep "Re: Re: Re: [PATCH 1/5 v2] third"
'
test_expect_success 'am -3 falls back to 3-way merge' '
diff --git a/t/t6040-tracking-info.sh b/t/t6040-tracking-info.sh
index aac212e..ba90601 100755
--- a/t/t6040-tracking-info.sh
+++ b/t/t6040-tracking-info.sh
@@ -53,7 +53,7 @@ test_expect_success 'checkout' '
(
cd test && git checkout b1
) >actual &&
- grep -e "have 1 and 1 different" actual
+ grep "have 1 and 1 different" actual
'
test_expect_success 'status' '
@@ -63,7 +63,7 @@ test_expect_success 'status' '
# reports nothing to commit
test_must_fail git status
) >actual &&
- grep -e "have 1 and 1 different" actual
+ grep "have 1 and 1 different" actual
'
diff --git a/t/t7002-grep.sh b/t/t7002-grep.sh
index 5e359cb..18fe6f2 100755
--- a/t/t7002-grep.sh
+++ b/t/t7002-grep.sh
@@ -109,7 +109,7 @@ do
'
test_expect_success "grep -c $L (no /dev/null)" '
- ! git grep -c test $H | grep -q /dev/null
+ ! git grep -c test $H | grep /dev/null
'
done
diff --git a/t/t7701-repack-unpack-unreachable.sh b/t/t7701-repack-unpack-unreachable.sh
index 531dac0..b48046e 100755
--- a/t/t7701-repack-unpack-unreachable.sh
+++ b/t/t7701-repack-unpack-unreachable.sh
@@ -29,7 +29,7 @@ test_expect_success '-A option leaves unreachable objects unpacked' '
git repack -A -d -l &&
# verify objects are packed in repository
test 3 = $(git verify-pack -v -- .git/objects/pack/*.idx |
- grep -e "^$fsha1 " -e "^$csha1 " -e "^$tsha1 " |
+ egrep "^($fsha1|$csha1|$tsha1) " |
sort | uniq | wc -l) &&
git show $fsha1 &&
git show $csha1 &&
@@ -41,7 +41,7 @@ test_expect_success '-A option leaves unreachable objects unpacked' '
git repack -A -d -l &&
# verify objects are retained unpacked
test 0 = $(git verify-pack -v -- .git/objects/pack/*.idx |
- grep -e "^$fsha1 " -e "^$csha1 " -e "^$tsha1 " |
+ egrep "^($fsha1|$csha1|$tsha1) " |
sort | uniq | wc -l) &&
git show $fsha1 &&
git show $csha1 &&
diff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh
index 46ba19b..07117a8 100755
--- a/t/t9500-gitweb-standalone-no-errors.sh
+++ b/t/t9500-gitweb-standalone-no-errors.sh
@@ -56,7 +56,7 @@ gitweb_run () {
rm -f gitweb.log &&
perl -- "$TEST_DIRECTORY/../gitweb/gitweb.perl" \
>/dev/null 2>gitweb.log &&
- if grep -q -s "^[[]" gitweb.log >/dev/null; then false; else true; fi
+ if grep "^[[]" gitweb.log >/dev/null 2>&1; then false; else true; fi
# gitweb.log is left for debugging
}
--
1.6.0.2.517.g4d51
^ permalink raw reply related
* Re: [PATCH] git-gui: Do not automatically stage file after merge tool finishes
From: Alexander Gavrilov @ 2008-09-30 8:00 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Shawn O. Pearce, Git Mailing List
In-Reply-To: <48E1CA7F.5050501@viscovery.net>
On Tuesday 30 September 2008 10:43:11 Johannes Sixt wrote:
> From: Johannes Sixt <johannes.sixt@telecom.at>
>
> If a merge tool was invoked on a conflicted file and the tool completed,
> then the conflicted file was staged automatically. However, the fact that
> the user closed the merge tool cannot be understood as the unequivocal
> sign that the conflict was completely resolved. For example, the user
> could have decided to postpone the resolution of the conflict, or could
> have accidentally closed the tool. We better leave the file unstaged and
> let the user stage it explicitly.
>
> Since the file is not staged anyway, the check for an unmodified
> timestamp is pointless and removed.
>
> Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
> ---
> I had sent this patch last week (but marked as RFC). Here it is again
> without 'RFC' because I think it is a necessary change.
Now that the issue with staging of working copy files is more or less
resolved, I agree that it is better to disable automatic staging.
Alexander
^ permalink raw reply
* Re: [RFC PATCH (GIT-GUI)] git-gui: Add more integration options to citool.
From: Alexander Gavrilov @ 2008-09-30 7:53 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git
In-Reply-To: <20080926144536.GA3669@spearce.org>
On Friday 26 September 2008 18:45:36 Shawn O. Pearce wrote:
> Alexander Gavrilov <angavrilov@gmail.com> wrote:
> > On Wednesday 24 September 2008 20:52:01 Shawn O. Pearce wrote:
> > >
> > > --8<--
> > > git-gui: Hide commit related UI during citool --nocommit
> >
> > I believe that the 'Sign Off' button should better be controlled by the nocommitmsg
> > option; otherwise this looks good to me. But I'm not the best thinker at the
> > moment (had a cold).
>
> Since I've already pushed that patch in both git-gui.git and git.git
> how is this as a followup?
Yes, it does what I was thinking of.
> --8<--
> git-gui: Show/hide "Sign Off" based on nocommitmsg option
>
> If citool --nocommit is invoked we hide the Sign Off features, as
> the commit message area is not editable. But we really want the
> selection tied to the message area's editing ability.
>
> Suggested-by: Alexander Gavrilov <angavrilov@gmail.com>
> Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
> ---
> git-gui.sh | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/git-gui.sh b/git-gui.sh
> index 4085e8f..09ce410 100755
> --- a/git-gui.sh
> +++ b/git-gui.sh
> @@ -2413,7 +2413,7 @@ if {[is_enabled multicommit] || [is_enabled singlecommit]} {
>
> .mbar.commit add separator
>
> - if {![is_enabled nocommit]} {
> + if {![is_enabled nocommitmsg]} {
> .mbar.commit add command -label [mc "Sign Off"] \
> -command do_signoff \
> -accelerator $M1T-S
> @@ -2743,7 +2743,7 @@ pack .vpane.lower.commarea.buttons.incall -side top -fill x
> lappend disable_on_lock \
> {.vpane.lower.commarea.buttons.incall conf -state}
>
> -if {![is_enabled nocommit]} {
> +if {![is_enabled nocommitmsg]} {
> button .vpane.lower.commarea.buttons.signoff -text [mc "Sign Off"] \
> -command do_signoff
> pack .vpane.lower.commarea.buttons.signoff -side top -fill x
> --
> 1.6.0.2.508.g2bf53a
>
>
^ permalink raw reply
* Re: What's cooking in git/spearce.git (Sep 2008, #04; Mon, 22)
From: Michael J Gruber @ 2008-09-30 7:52 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git
In-Reply-To: <20080929184709.GB21310@spearce.org>
Shawn O. Pearce venit, vidit, dixit 29.09.2008 20:47:
> Here are the topics that have been cooking. Commits prefixed
> with '-' are only in 'pu' while commits prefixed with '+' are
> in 'next'.
...
> * mg/maint-remote-fix (Mon Sep 22 10:57:51 2008 +0200) 1 commit
> + make "git remote" report multiple URLs
>
> (Tip at 7d20e21)
>
> * bc/maint-diff-hunk-header-fix (Sat Sep 20 15:30:12 2008 -0700) 5 commits
> + diff hunk pattern: fix misconverted "\{" tex macro introducers
> + diff: use extended regexp to find hunk headers
> + diff.*.xfuncname which uses "extended" regex's for hunk header
> selection
> + diff.c: associate a flag with each pattern and use it for
> compiling regex
> + diff.c: return pattern entry pointer rather than just the hunk
> header pattern
>
> (Tip at 96d1a8e)
>
> The above two are ready for 'maint'.
They are already in maint, as per "What's in..." and the repo.
Cheers,
Michael
^ permalink raw reply
* Re: [PATCH 2/6] gitweb: use_pathinfo filenames start with /
From: Giuseppe Bilotta @ 2008-09-30 7:48 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git, Petr Baudis, Lea Wiemann
In-Reply-To: <200809300120.02492.jnareb@gmail.com>
On Tue, Sep 30, 2008 at 1:20 AM, Jakub Narebski <jnareb@gmail.com> wrote:
>
> Hn. Now I am not sure if it should be squashed, or should be separate.
Yeah. The fact that it's *specifically* to allow web docs to be used
in raw view makes it count towarda a feature in itself, even if the
patch by itself is trivial ...
So. Squashed or separate? :)
--
Giuseppe "Oblomov" Bilotta
^ permalink raw reply
* [JGIT PATCH 2/2] ObjectId: Convert StringIndexOutOfBounds to IllegalArgumentException
From: Shawn O. Pearce @ 2008-09-30 7:47 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: git
In-Reply-To: <1222760871-58768-1-git-send-email-spearce@spearce.org>
If parsing an ObjectId fromString fails because the string is too short
we don't want to throw StringIndexOutOfBoundsException with a trace that
leads back to the error handling code; instead we should throw what the
caller expects us to throw, which is IllegalArgumentException.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
.../src/org/spearce/jgit/lib/MutableObjectId.java | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/MutableObjectId.java b/org.spearce.jgit/src/org/spearce/jgit/lib/MutableObjectId.java
index f88d8cb..5b16345 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/MutableObjectId.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/MutableObjectId.java
@@ -161,6 +161,8 @@ private void fromHexString(final byte[] bs, int p) {
throw new IllegalArgumentException("Invalid id: " + str);
} catch (UnsupportedEncodingException e2) {
throw new IllegalArgumentException("Invalid id");
+ } catch (StringIndexOutOfBoundsException e2) {
+ throw new IllegalArgumentException("Invalid id");
}
}
}
--
1.6.0.2.463.g7f0eb
^ permalink raw reply related
* [JGIT PATCH 1/2] Fix the UnpackedObjectCache hash function to prevent overflow
From: Shawn O. Pearce @ 2008-09-30 7:47 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: git
Sometimes we got negative array indexes inside of the cache due
to WindowedFile.hash having a negative value assigned by the JVM.
We now use only the lower 8 bits as the cache is fixed to at most
256 entries.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
.../org/spearce/jgit/lib/UnpackedObjectCache.java | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/UnpackedObjectCache.java b/org.spearce.jgit/src/org/spearce/jgit/lib/UnpackedObjectCache.java
index 03cf674..ee6a680 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/UnpackedObjectCache.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/UnpackedObjectCache.java
@@ -48,9 +48,9 @@
private static int hash(final WindowedFile pack, final long position) {
int h = pack.hash + (int) position;
- h += h >> 16;
- h += h >> 8;
- return h % CACHE_SZ;
+ h += h >>> 16;
+ h += h >>> 8;
+ return h & (CACHE_SZ - 1);
}
private static int maxByteCount;
--
1.6.0.2.463.g7f0eb
^ permalink raw reply related
* Re: [PATCH] Builtin-commit: show on which branch a commit was added
From: Jeff King @ 2008-09-30 7:09 UTC (permalink / raw)
To: Wincent Colaiuta
Cc: Andreas Ericsson, Pieter de Bie, Junio C Hamano, Git Mailinglist
In-Reply-To: <836C204F-F5AF-4887-99C9-04E70FEEB998@wincent.com>
On Tue, Sep 30, 2008 at 08:37:00AM +0200, Wincent Colaiuta wrote:
>> "commit" is just noise.
>
> Excellent point on the noise. Independently of whether the branch info
> gets added the word "commit" should probably be dropped.
The branch info has already been added, if you count it being in next
(in the form of "on $branch: ").
> As far as long-line-wrapping goes, I don't really think this is a problem
> for Git to solve (by truncation or any other means); it's more of a user
> behaviour thing where one would hope that users would get into the habit
> of using concise subject lines and branch names.
How concise must we be? I wrap my commit messages at 60 characters,
which I consider quite conservative. But
Created commit abcd1234 on jk/my-topic-branch:
takes up over half of an 80-column terminal. Is that a long branch name?
Browsing "git log --grep=Merge.branch --pretty=format:%s origin/next"
suggests it's not terribly out of line (at least by Junio's standards).
Dropping "commit " will help some. But given how much width is still
used, and the fact that this message is really just to say "yes, I
confirm that we just created the commit you asked for", I think
truncating (with dots) to keep it within 80 characters is reasonable.
-Peff
^ permalink raw reply
* [PATCH] git-gui: Do not automatically stage file after merge tool finishes
From: Johannes Sixt @ 2008-09-30 6:43 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Alexander Gavrilov, Git Mailing List
From: Johannes Sixt <johannes.sixt@telecom.at>
If a merge tool was invoked on a conflicted file and the tool completed,
then the conflicted file was staged automatically. However, the fact that
the user closed the merge tool cannot be understood as the unequivocal
sign that the conflict was completely resolved. For example, the user
could have decided to postpone the resolution of the conflict, or could
have accidentally closed the tool. We better leave the file unstaged and
let the user stage it explicitly.
Since the file is not staged anyway, the check for an unmodified
timestamp is pointless and removed.
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
I had sent this patch last week (but marked as RFC). Here it is again
without 'RFC' because I think it is a necessary change.
-- Hannes
lib/mergetool.tcl | 10 +---------
1 files changed, 1 insertions(+), 9 deletions(-)
diff --git a/lib/mergetool.tcl b/lib/mergetool.tcl
index 6ab5701..8d1ee5b 100644
--- a/lib/mergetool.tcl
+++ b/lib/mergetool.tcl
@@ -375,14 +375,6 @@ proc merge_tool_finish {fd} {
}
}
- # Check the modification time of the target file
- if {!$failed && [file mtime $mtool_target] eq $mtool_mtime} {
- if {[ask_popup [mc "File %s unchanged, still accept as resolved?" \
- [short_path $mtool_target]]] ne {yes}} {
- set failed 1
- }
- }
-
# Finish
if {$failed} {
file rename -force -- $backup $mtool_target
@@ -395,6 +387,6 @@ proc merge_tool_finish {fd} {
delete_temp_files $mtool_tmpfiles
- merge_add_resolution $mtool_target
+ reshow_diff
}
}
--
1.6.0.2.1262.ge466e
^ permalink raw reply related
* [PATCH] git-gui: Remove space from the end of aspell's reply before processing
From: Johannes Sixt @ 2008-09-30 6:39 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Git Mailing List
From: Johannes Sixt <johannes.sixt@telecom.at>
When git gui processes a reply from aspell it explicitly ignores an empty
line. The Windows version of aspell, however, terminates lines with CRLF,
but TCL's 'gets' does not remove CR, hence, a "visibly" empty line was not
actually recognized as empty. With this change we explicitly trim off
whitespace before the line is further processed.
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
There's probably a better solution by having TCL do the translation of
line endings, but I don't know how to do that.
-- Hannes
lib/spellcheck.tcl | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/lib/spellcheck.tcl b/lib/spellcheck.tcl
index a479b2f..e612030 100644
--- a/lib/spellcheck.tcl
+++ b/lib/spellcheck.tcl
@@ -314,6 +314,7 @@ method _run {} {
method _read {} {
while {[gets $s_fd line] >= 0} {
set lineno [lindex $s_pending 0 0]
+ set line [string trim $line]
if {$s_clear} {
$w_text tag remove misspelled "$lineno.0" "$lineno.end"
--
1.6.0.2.1262.ge466e
^ permalink raw reply related
* Re: [PATCH] Builtin-commit: show on which branch a commit was added
From: Wincent Colaiuta @ 2008-09-30 6:37 UTC (permalink / raw)
To: Andreas Ericsson
Cc: Jeff King, Pieter de Bie, Junio C Hamano, Git Mailinglist
In-Reply-To: <48E1C39F.4070906@op5.se>
El 30/9/2008, a las 8:13, Andreas Ericsson escribió:
> Jeff King wrote:
>> On Mon, Sep 29, 2008 at 10:09:17PM +0200, Pieter de Bie wrote:
>>> How about something like
>>>
>>> Created commit abcd1234 on widget -- "subwidget: one line summary"
>> I think that is probably just trading one visual problem for another.
>> That is, there are other people will have the same problem with "--"
>> that I had with ": ".
>
> Created 6207abc (subwidget: one quite long line of sum...) on <branch>
>
> "commit" is just noise.
Excellent point on the noise. Independently of whether the branch info
gets added the word "commit" should probably be dropped.
As far as long-line-wrapping goes, I don't really think this is a
problem for Git to solve (by truncation or any other means); it's more
of a user behaviour thing where one would hope that users would get
into the habit of using concise subject lines and branch names.
Cheers,
Wincent
^ permalink raw reply
* Re: [PATCH] Builtin-commit: show on which branch a commit was added
From: Jeff King @ 2008-09-30 6:16 UTC (permalink / raw)
To: Andreas Ericsson; +Cc: Pieter de Bie, Junio C Hamano, Git Mailinglist
In-Reply-To: <48E1C39F.4070906@op5.se>
On Tue, Sep 30, 2008 at 08:13:51AM +0200, Andreas Ericsson wrote:
> Created 6207abc (subwidget: one quite long line of sum...) on <branch>
>
> "commit" is just noise. Parentheses are often used to extemporize when
> using normal written language so it should work well here too.
I do like that better, and there is some precedent in the way we mention
commits in emails (I know Junio even has an alias that formats it as
$hash ($subject)).
By your "..." are you suggesting to truncate the subject?
-Peff
^ permalink raw reply
* Re: [PATCH] Builtin-commit: show on which branch a commit was added
From: Andreas Ericsson @ 2008-09-30 6:13 UTC (permalink / raw)
To: Jeff King; +Cc: Pieter de Bie, Junio C Hamano, Git Mailinglist
In-Reply-To: <20080929224430.GA11545@sigill.intra.peff.net>
Jeff King wrote:
> On Mon, Sep 29, 2008 at 10:09:17PM +0200, Pieter de Bie wrote:
>
>> How about something like
>>
>> Created commit abcd1234 on widget -- "subwidget: one line summary"
>
> I think that is probably just trading one visual problem for another.
> That is, there are other people will have the same problem with "--"
> that I had with ": ".
>
Created 6207abc (subwidget: one quite long line of sum...) on <branch>
"commit" is just noise. Parentheses are often used to extemporize when
using normal written language so it should work well here too.
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ 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