* Re: [PATCH] Make "git stash" configurable
From: Junio C Hamano @ 2007-12-21 19:48 UTC (permalink / raw)
To: Steven Grimm
Cc: しらいしななこ, git
In-Reply-To: <2AB285BD-DA6E-49D1-B664-B1A8B552DD76@midwinter.com>
Steven Grimm <koreth@midwinter.com> writes:
> On Dec 21, 2007, at 2:22 AM, しらいしななこ wrote:
>
>> "git stash" without argument originally created an unnamed
>> stash, but some people felt this can be confusing to new
>> users. This introduces per-user config variable stash.quick to
>> control this behavior.
>
> This whole thing smells of indecision to me.
If you want to force me to decide, you know what the decision it
would be. Here are the guiding principles and decisions based
on them.
(1) A tool should be optimized for people who know what it does
and how to use it.
While forbidding "git stash" without argument and forcing
people who want to have a quick stash to type "git stash
save" may appear not to be adding too many keystrokes, it
actually is a regression. Because "git stash save" without
arguments make a much less descriptive stash than the
current quickie "git stash", doing so slows them down by
forcing them to think of what message to give.
The decision would be to keep "git stash" quickie stash
creation. An alternative decision could be to do nothing
when "git stash" is given, and make "git stash save" (and
"git stash create") without the actual message to come up
with a better default message.
(2) A tool should not change its behaviour too drastically
depending on who the user is.
Otherwise it makes it harder for people with mid-level
experience to teach less experienced people. The decision
would be that "stash.quick=ask" and "stash.quick=no" are
unacceptable.
(3) A tool should support safety for a mistaken use by who know
the tool (even they can make mistakes).
I listed this mostly because we made "git clean" to violate
principle (2) above. Even when you know what that command
does, it is easy to mean "make clean" and type "git clean"
by mistake. And "git clean" is very destructive --- there
is no "oops, I take it back". This principle has led us to
introduce clean.requireforce that defaults to true.
For "git stash", there is no "make stash" or "xyzzy stash"
that can easily induce mistyping. Even if it were typed by
mistake, there is no such destructive "oops, I cannot take
it back" problem with "git stash", either. "git stash
apply" will get you back to where you were. The decision
would again be that "stash.quick=no" is unwanted.
(4) A tool should support safety for clueless people when it is
reasonable.
Even though "I did not know what command foo does, so I
tried running it and it did something unexpected" is a
silly excuse to rob quickie "git stash" from people who
know better, we cannot avoid the fact that there are
clueless people. I think checking stash.iknowwhatitdoes to
detect first-time users, and explaining what it does to
them, may make sense. And we can take hints from the patch
that started this thread how to do this.
The decision here is that I am open to a change that
implements the one-time safety instruction.
Note that in the above guiding principles, I used "should" in
RFC2119/BCP0014 sense: there may exist valid reasons in
particular circumstances to ignore a particular item, but the
full implications must be understood and carefully weighed
before choosing a different course.
^ permalink raw reply
* Re: msysgit Git-1.5.4-rc1 available
From: Steffen Prohaska @ 2007-12-21 19:25 UTC (permalink / raw)
To: Git Mailing List, msysGit
In-Reply-To: <0F6939E0-686C-4526-85D1-447C080230A5-wjoc1KHpMeg@public.gmane.org>
On Dec 21, 2007, at 11:05 AM, Steffen Prohaska wrote:
> for download from
>
> http://msysgit.googlecode.com/files/Git-1.5.4-rc1-preview20071221.exe
I recognized that this installer does not contain
all new binaries.
I uploaded a corrected version
Git-1.5.4-rc1-preview20071221-corrected.exe
which is available from
http://code.google.com/p/msysgit/downloads
Sorry for the confusion.
Steffen
^ permalink raw reply
* Re: [PATCH v2] Make git send-email accept $EDITOR with arguments
From: Jeff King @ 2007-12-21 19:23 UTC (permalink / raw)
To: Gustaf Hendeby; +Cc: luciano, git, gitster
In-Reply-To: <476BDA5F.4070306@isy.liu.se>
On Fri, Dec 21, 2007 at 04:23:11PM +0100, Gustaf Hendeby wrote:
> > If you are going to do it that way, I suspect you want to quotemeta
> > $compose_filename.
> Generally that would be true, but is that really necessary when I know
> $compose_filename is defined as:
>
> my $compose_filename = ".msg.$$";
I know; it is just easier to see that it is correct with the quotemeta
(and correct in the face of somebody changing the message later).
> Or, should I take it that you prefer the version using split? I didn't
> really feel good about the possibility of splitting paths with spaces
> that came with that one though.
I am fine with using the shell. Though keep in mind that the two
solutions will behave differently with
EDITOR='foo; bar'
That is, system("$editor $message") will actually invoke the shell,
whereas system(split(/ /, $editor), $message) will _just_ split on
whitespace. We should do whatever is consistent with the rest of the git
commands (off the top of my head, I don't know).
-Peff
^ permalink raw reply
* Re: [PATCH] git-send-email: Add --suppress-all-from option.
From: Joel Becker @ 2007-12-21 19:21 UTC (permalink / raw)
To: Junio C Hamano; +Cc: David Brown, git
In-Reply-To: <7vfxxw7xkb.fsf@gitster.siamese.dyndns.org>
On Fri, Dec 21, 2007 at 09:43:48AM -0800, Junio C Hamano wrote:
> The option name feels as if it is somehow affecting From: but
> this is all about recipients. It needs to be named better.
>
> Even more importantly, git-send-email has too many places that
> pick up additional recipients. I doubt --suppress-foo to
> suppress one such source "foo" is sustainable. We should try to
> clean up the mess, not adding to it.
Yay, even better that we're going to evaluate the sucker (I was
just complaining about this yesterday to someone, so how apropos that it
comes up on-list).
First and foremost, I think git-send-email should not default to
anything. It was quite a surprise, the first time I tried to use it, to
discover I had to add two options to ~/.gitconfig just for sane
behavior. Never mind that I couldn't suppress the author-cc. I think
that a naive "git send-email --to bob@bob.com foo.patch" should only go
to bob, period.
We can then add ways to auto-cc. I don't mind typing the extra
bits. Heck, we could even define a --review that does what is currently
the default - cc-everyone-who-might-care-as-we-go-upstream.
Joel
--
Life's Little Instruction Book #99
"Think big thoughts, but relish small pleasures."
Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker@oracle.com
Phone: (650) 506-8127
^ permalink raw reply
* Re: [PATCH] Make "git stash" configurable
From: Steven Grimm @ 2007-12-21 18:26 UTC (permalink / raw)
To: Junio C Hamano
Cc: Matthieu Moy,
しらいしななこ, git
In-Reply-To: <7v3atw7x33.fsf@gitster.siamese.dyndns.org>
On Dec 21, 2007, at 9:54 AM, Junio C Hamano wrote:
> Matthieu Moy <Matthieu.Moy@imag.fr> writes:
>> It also makes portable scripts harder. It's easy to write a script
>> that works only if you have the option set ...
>
> In general I'd agree but in this particular case I do not think
> that applies. Scripts can always say what they want, either
> "stash save" or "stash list".
I bet Matthieu meant that it's easy for someone who runs bare "git
stash" all the time to forget that there's an option to make that
command do something different, and to thus code up a local script
that runs the command as they're used to running it. Such a script
will work just fine for them but it'll break as soon as they give it
to someone else who has the option set differently.
-Steve
^ permalink raw reply
* Re: [PATCH] Make "git stash" configurable
From: Junio C Hamano @ 2007-12-21 17:54 UTC (permalink / raw)
To: Matthieu Moy
Cc: Steven Grimm,
しらいしななこ, git
In-Reply-To: <vpq4pec7yb8.fsf@bauges.imag.fr>
Matthieu Moy <Matthieu.Moy@imag.fr> writes:
> Steven Grimm <koreth@midwinter.com> writes:
>
>> Git's command line interface is already confusing enough without
>> adding this kind of variability
>
> It also makes portable scripts harder. It's easy to write a script
> that works only if you have the option set ...
In general I'd agree but in this particular case I do not think
that applies. Scripts can always say what they want, either
"stash save" or "stash list".
^ permalink raw reply
* Re: [PATCH] git-describe: Add a --match option to limit considered tags.
From: Junio C Hamano @ 2007-12-21 17:52 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: git
In-Reply-To: <1198253900-31502-1-git-send-email-madcoder@debian.org>
Does it work with "describe --contains" as well?
^ permalink raw reply
* Re: [PATCH v0] sha1_name: grok <revision>:./<relative-path>
From: Junio C Hamano @ 2007-12-21 17:50 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy
Cc: Johannes Schindelin, Dana How, Linus Torvalds, Alex Riesen,
Jakub Narebski, git
In-Reply-To: <fcaeb9bf0712210617x2bafa33cp15815a59fc631f45@mail.gmail.com>
"Nguyen Thai Ngoc Duy" <pclouds@gmail.com> writes:
>> Note that this patch does not handle "../", and neither do I plan to.
>
> Junio's rc1 announcement got me to read this. It would be indeed
> useful as I usually work in deep subdirs. However, from my user
> perspective, the right approach is to make <treeish>:path always be
> relative to current directory. If you want absolute path, use
> <treeish>:/path. More intuitive but it breaks current behavior.
I do not know if you followed the discussion thread, but the
<treeish>:relative-path has been shown to be broken semantics,
so even if it may be "intuitive", it is intuitive only to people
who do not understand the brokenness. Please read the one that
Dscho talks about windows drive letter and Linus agrees that is
a good analogy.
It might be possible to do <commit>:relative and apply that only
to direct user input, but I do not think it is worth the
compatibility and complexity hassle.
^ permalink raw reply
* Re: Linux 2.6.24-rc6
From: Linus Torvalds @ 2007-12-21 17:45 UTC (permalink / raw)
To: Junio C Hamano, Git Mailing List
In-Reply-To: <alpine.LFD.0.9999.0712202110350.21557@woody.linux-foundation.org>
On Thu, 20 Dec 2007, Linus Torvalds wrote:
>
> Both answers are *correct*, though. The particular choice of "insert at
> line 489, after line 488" is a bit odd, but is because we don't actually
> search to exactly the beginning of where the differences started, we
> search in blocks of 1kB and then we go forward to the next newline.
This slightly more involved diff does a better job at this particular
issue. Whether the complexity is worth it or not, I dunno, but it changes
the "remove common lines at the end" to do an exact job, which for this
particular test-case means that the end result of adding a thousand lines
of 'y' will look like
[torvalds@woody ~]$ git diff -U0 a b | grep @@
@@ -0,0 +1,1000 @@
instead - ie it will say that they were added at the very beginning of the
file rather than added at some arbitrary point in the middle.
Whether this is really worth it, I dunno.
Also, I'm kind of debating with myself whether it would make most sense to
only do this kind of optimization when (pick arbitrary cut-off here)
something like more than half of the file is identical at the end. If we
don't have a noticeable fraction of the file being the same, it may not
make sense to really bother with this, since it really is meant for just
things like ChangeLog files etc that have data added at the beginning.
That would make this whole optimization a lot more targeted to the case
where it really matters and really helps.
I also do have an incling of a really evil way to make xdiff handle the
case of having multiple lines of context right too, and basically just
move all of this logic into xdiff itself rather than have this
interface-level hack, but I'll have to let that idea brew for a while yet.
Linus
---
xdiff-interface.c | 38 ++++++++++++++++++++++++++++++--------
1 files changed, 30 insertions(+), 8 deletions(-)
diff --git a/xdiff-interface.c b/xdiff-interface.c
index 9ee877c..54a53d2 100644
--- a/xdiff-interface.c
+++ b/xdiff-interface.c
@@ -109,21 +109,43 @@ int xdiff_outf(void *priv_, mmbuffer_t *mb, int nbuf)
*/
static void trim_common_tail(mmfile_t *a, mmfile_t *b, long ctx)
{
- const int blk = 1024;
+ int blk = 1024;
long trimmed = 0, recovered = 0;
char *ap = a->ptr + a->size;
char *bp = b->ptr + b->size;
long smaller = (a->size < b->size) ? a->size : b->size;
- while (blk + trimmed <= smaller && !memcmp(ap - blk, bp - blk, blk)) {
- trimmed += blk;
- ap -= blk;
- bp -= blk;
- }
+ if (ctx)
+ return;
+
+ do {
+ while (blk + trimmed <= smaller && !memcmp(ap - blk, bp - blk, blk)) {
+ trimmed += blk;
+ ap -= blk;
+ bp -= blk;
+ }
+ blk /= 2;
+ } while (blk);
+
+ /* Did we trim one of them all away? */
+ if (trimmed == smaller) {
+ char *bigger;
+ if (a->size == b->size)
+ return;
+ bigger = a->ptr;
+ if (a->size > b->size)
+ bigger = b->ptr;
+
+ /* Did the other one end in a newline? */
+ if (bigger[trimmed-1] == '\n')
+ goto done;
+ }
- while (recovered < trimmed && 0 <= ctx)
+ /* Find the next newline */
+ while (recovered < trimmed)
if (ap[recovered++] == '\n')
- ctx--;
+ break;
+done:
a->size -= (trimmed - recovered);
b->size -= (trimmed - recovered);
}
^ permalink raw reply related
* Re: [PATCH] git-send-email: Add --suppress-all-from option.
From: Junio C Hamano @ 2007-12-21 17:43 UTC (permalink / raw)
To: David Brown; +Cc: git
In-Reply-To: <1198216860-487-1-git-send-email-git@davidb.org>
David Brown <git@davidb.org> writes:
> Sometimes, it is useful to be able to send a patch to a third party
> without the author of the patch being copied on the message.
I would agree with the cause, but not necessarily with the
execution.
> +--suppress-all-from, --no-suppress-all-from::
> + If this is set, do not add the From: address to the cc: list,
> + even if it is different than the person sending the email.
> + Default is the value of the 'sendemail.suppressallfrom'
> + configuration value; if that is unspecified, default to
> + -no-suppress-all-from.
The option name feels as if it is somehow affecting From: but
this is all about recipients. It needs to be named better.
Even more importantly, git-send-email has too many places that
pick up additional recipients. I doubt --suppress-foo to
suppress one such source "foo" is sustainable. We should try to
clean up the mess, not adding to it.
So let's analyze the current situation first. It seems that we
currently pick up the list of recipients from the following
places:
* obviously, --to command line;
* mbox Cc: header lines;
* mbox From: header lines;
* lots-of-email first line
* $cc_cmd output;
* Signed-off-by: lines in body;
* Cc: lines in body;
The --no-signed-off-cc option is about omitting the last two
from the recipients. We do not have a way to squelch other
sources of extra recipients, hence the need for your patch.
The --suppress-from option is about not giving an extra copy to
the sender. It is "suppress from-address from the recipient
list", so the option name makes sense.
Your --suppress-all-from, from a cursory read of your patch,
omits only mbox Cc: and From: line recipients -- it is far from
"all", isn't it? --signed-off-cc defaults to true so you would
need to suppress that at least to call it "all".
A cleaner approach might be:
- introduce a helper function add_to_recipients that take \@cc,
$recipient and the "source class". Make this function
responsible for not adding the sender to the list
(i.e. --suppress-from, which is currently checked
everywhere), and for not adding recipients from specified
classes of sources, like this:
sub add_to_recipients {
my ($cc, $source, $recipient) = @_;
return 0 if ($suppress_from and $sender eq $recipient);
return 0 if ($suppressed_recipient_source{$source});
push @$cc, $recipient;
return 1;
}
Instead of returning 1 unconditionally, it might make sense
to omit pushing duplicate here and to return 0 when
$recipient was already in @$cc.
- adjust the places where "push @cc" happens to use the above
helper; the existing suppress logic in the callers can and
should be removed as the add_to_recipients will be
responsible for it, like this (an example for cc-cmd part):
if (defined $cc_cmd) {
open(F, "$cc_cmd $t |")
or die "(cc-cmd) Could not execute '$cc_cmd'";
while(<F>) {
my $c = $_;
$c =~ s/^\s*//g;
$c =~ s/\n$//g;
- next if ($c eq $sender and $suppress_from);
- push @cc, $c;
+ next if (!add_to_recipients(\@cc, 'cccmd', $c));
printf("(cc-cmd) Adding cc: %s from: '%s'\n",
$c, $cc_cmd) unless $quiet;
}
- define a global %suppressed_recipient_source hash to be used
in add_to_recipients(). The existing --no-signed-off-cc is
about adding two sources to this hash.
- make the %suppressed_recipient_source configurable from the
command line and repository configuration.
As to the "recipient source" classes, I think they can be
categorized as:
* 'cc', to cover mbox Cc: header, lots-of-email first line, and
Cc: lines in body;
* 'sob', to cover Signed-off-by: lines in body;
* 'cccmd', to cover $cc_cmd output;
Hmm?
^ permalink raw reply
* [PATCH] Allow selection of different cleanup modes for commit messages
From: Alex Riesen @ 2007-12-21 17:35 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.LFD.0.9999.0712201555570.21557@woody.linux-foundation.org>
The modes being default, verbatim, whitespace and strip. The default
mode depends on if the message is being edited and will either strip
whitespace and comments (if editor active) or just strip the
whitespace (for where the message is given explicitely).
Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
---
Linus Torvalds, Fri, Dec 21, 2007 01:03:26 +0100:
> But for the case of actually using the built-in editing capability, we
> don't have that choice, so I'd argue that regardless, we should make the
> "--verbatim" switch be a three-way choice between (a) not touching things
> at all, (b) touching up just whitespace, or (c) doing the full enchilada.
>
> And if we introduce such a flag, then I think we can make the *default*
> (in the absense of an explicit flag) be something like
>
> if (no_edit)
> default = touch_up_just_whitespace;
> else
> default = whole_enchilada
>
> and obviously it could also be a configuration option.
which makes it four-way choice...
> That way, you could always get the existing behaviour with
>
> git commit --verbatim=full-enchilada -F my_message
>
> (yeah, bad name - I'm not seriously suggesting it be called
> "full-enchilada", and I'm also not sure the flag should be called
> "--verbatim" any more if it has choices, it's more likely that we should
> call it something like "--cleanup={verbatim,whitespace,strip}" or
> something like that.
"verbatim", "whitespace", "strip", "default".
The patch is on top of my previos patches. Junio, if you wish, I can
squash and resend.
Documentation/git-commit.txt | 15 ++++++---
builtin-commit.c | 69 +++++++++++++++++++++++++++++-------------
t/t7502-commit.sh | 28 +++++++++++++++--
3 files changed, 83 insertions(+), 29 deletions(-)
diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 862543f..ac2bfd5 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -11,7 +11,7 @@ SYNOPSIS
'git-commit' [-a | --interactive] [-s] [-v] [-u]
[(-c | -C) <commit> | -F <file> | -m <msg> | --amend]
[--allow-empty] [--no-verify] [-e] [--author <author>]
- [--verbatim] [--] [[-i | -o ]<file>...]
+ [--cleanup=<mode>] [--] [[-i | -o ]<file>...]
DESCRIPTION
-----------
@@ -95,10 +95,15 @@ OPTIONS
from making such a commit. This option bypasses the safety, and
is primarily for use by foreign scm interface scripts.
---verbatim::
- Inhibits stripping of leading and trailing spaces,
- empty lines and #commentary from the commit message.
- Implies --allow-empty.
+--cleanup=<mode>::
+ This option sets how the commit message is cleaned up.
+ The '<mode>' can be one of 'verbatim', 'whitespace', 'strip',
+ and 'default'. The 'default' mode will strip leading and
+ trailing empty lines and #commentary from the commit message
+ only if the message is to be edited. Otherwise only whitespace
+ removed. The 'verbatim' mode wont change message at all,
+ 'whitespace' removes just leading/trailing whitespace lines
+ and 'strip' removes both whitespace and commentary.
-e|--edit::
The message taken from file with `-F`, command line with
diff --git a/builtin-commit.c b/builtin-commit.c
index eae7661..6f98537 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -47,7 +47,20 @@ static char *logfile, *force_author, *template_file;
static char *edit_message, *use_message;
static int all, edit_flag, also, interactive, only, amend, signoff;
static int quiet, verbose, untracked_files, no_verify, allow_empty;
-static int verbatim_message;
+/*
+ * The default commit message cleanup mode will remove the lines
+ * beginning with # (shell comments) and leading and trailing
+ * whitespaces (empty lines or containing only whitespaces)
+ * if editor is used, and only the whitespaces if the message
+ * is specified explicitly.
+ */
+static enum {
+ CLEANUP_DEFAULT,
+ CLEANUP_NONE,
+ CLEANUP_SPACE,
+ CLEANUP_ALL,
+} cleanup_mode;
+static char *cleanup_arg;
static int no_edit, initial_commit, in_merge;
const char *only_include_assumed;
@@ -89,7 +102,7 @@ static struct option builtin_commit_options[] = {
OPT_BOOLEAN(0, "amend", &amend, "amend previous commit"),
OPT_BOOLEAN(0, "untracked-files", &untracked_files, "show all untracked files"),
OPT_BOOLEAN(0, "allow-empty", &allow_empty, "ok to record an empty change"),
- OPT_BOOLEAN(0, "verbatim", &verbatim_message, "do not strip spaces and #comments from message"),
+ OPT_STRING(0, "cleanup", &cleanup_arg, "default", "how to strip spaces and #comments from message"),
OPT_END()
};
@@ -348,7 +361,7 @@ static int prepare_log_message(const char *index_file, const char *prefix)
if (fp == NULL)
die("could not open %s", git_path(commit_editmsg));
- if (!verbatim_message)
+ if (cleanup_mode != CLEANUP_NONE)
stripspace(&sb, 0);
if (signoff) {
@@ -397,21 +410,24 @@ static int prepare_log_message(const char *index_file, const char *prefix)
return !!DIFF_OPT_TST(&rev.diffopt, HAS_CHANGES);
}
- if (in_merge && !no_edit)
- fprintf(fp,
- "#\n"
- "# It looks like you may be committing a MERGE.\n"
- "# If this is not correct, please remove the file\n"
- "# %s\n"
- "# and try again.\n"
- "#\n",
- git_path("MERGE_HEAD"));
-
- if (!verbatim_message)
- fprintf(fp,
- "\n"
- "# Please enter the commit message for your changes.\n"
- "# (Comment lines starting with '#' will not be included)\n");
+ if (!no_edit) {
+ if (in_merge)
+ fprintf(fp,
+ "#\n"
+ "# It looks like you may be committing a MERGE.\n"
+ "# If this is not correct, please remove the file\n"
+ "# %s\n"
+ "# and try again.\n"
+ "#\n",
+ git_path("MERGE_HEAD"));
+ if (cleanup_mode != CLEANUP_NONE)
+ fprintf(fp,
+ "\n"
+ "# Please enter the commit message for your changes.\n");
+ if (cleanup_mode == CLEANUP_DEFAULT)
+ fprintf(fp,
+ "# (Comment lines starting with '#' will not be included)\n");
+ }
if (only_include_assumed)
fprintf(fp, "# %s\n", only_include_assumed);
@@ -435,7 +451,7 @@ static int message_is_empty(struct strbuf *sb, int start)
const char *nl;
int eol, i;
- if (verbatim_message && sb->len)
+ if (cleanup_mode == CLEANUP_NONE && sb->len)
return 0;
/* See if the template is just a prefix of the message. */
@@ -594,6 +610,16 @@ static int parse_and_validate_options(int argc, const char *argv[],
only_include_assumed = "Explicit paths specified without -i nor -o; assuming --only paths...";
also = 0;
}
+ if (!cleanup_arg || !strcmp(cleanup_arg, "default"))
+ cleanup_mode = CLEANUP_DEFAULT;
+ else if (!strcmp(cleanup_arg, "verbatim"))
+ cleanup_mode = CLEANUP_NONE;
+ else if (!strcmp(cleanup_arg, "whitespace"))
+ cleanup_mode = CLEANUP_SPACE;
+ else if (!strcmp(cleanup_arg, "strip"))
+ cleanup_mode = CLEANUP_ALL;
+ else
+ die("Invalid cleanup mode %s", cleanup_arg);
if (all && argc > 0)
die("Paths with -a does not make sense.");
@@ -820,8 +846,9 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
if (p != NULL)
strbuf_setlen(&sb, p - sb.buf + 1);
- if (!verbatim_message)
- stripspace(&sb, !no_edit);
+ if (cleanup_mode != CLEANUP_NONE)
+ stripspace(&sb, cleanup_mode == CLEANUP_DEFAULT ?
+ !no_edit: cleanup_mode == CLEANUP_ALL);
if (sb.len < header_len || message_is_empty(&sb, header_len)) {
rollback_index_files();
die("no commit message? aborting commit.");
diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh
index d549728..6219378 100755
--- a/t/t7502-commit.sh
+++ b/t/t7502-commit.sh
@@ -93,15 +93,37 @@ test_expect_success 'verbatim commit messages' '
echo >>negative &&
{ echo;echo "# text";echo; } >expect &&
- git commit --verbatim -t expect -a &&
+ git commit --cleanup=verbatim -t expect -a &&
git cat-file -p HEAD |sed -e "1,/^\$/d" |head -n 3 >actual &&
diff -u expect actual &&
echo >>negative &&
- git commit --verbatim -F expect -a &&
+ git commit --cleanup=verbatim -F expect -a &&
git cat-file -p HEAD |sed -e "1,/^\$/d">actual &&
diff -u expect actual &&
echo >>negative &&
- git commit --verbatim -m "$(cat expect)" -a &&
+ git commit --cleanup=verbatim -m "$(cat expect)" -a &&
+ git cat-file -p HEAD |sed -e "1,/^\$/d">actual &&
+ diff -u expect actual
+
+'
+
+test_expect_success 'cleanup only whitespace from commit messages' '
+
+ echo >>negative &&
+ { echo;echo "# text";echo; } >text &&
+ echo "# text" >expect &&
+ git commit --cleanup=whitespace -F text -a &&
+ git cat-file -p HEAD |sed -e "1,/^\$/d">actual &&
+ diff -u expect actual
+
+'
+
+test_expect_success 'cleanup commit messages' '
+
+ echo >>negative &&
+ { echo;echo "# text";echo sample;echo; } >text &&
+ echo sample >expect &&
+ git commit --cleanup=strip -F text -a &&
git cat-file -p HEAD |sed -e "1,/^\$/d">actual &&
diff -u expect actual
--
1.5.4.rc1.34.gf21f1
^ permalink raw reply related
* Re: [PATCH] Make "git stash" configurable
From: Matthieu Moy @ 2007-12-21 17:27 UTC (permalink / raw)
To: Steven Grimm
Cc: しらいしななこ,
Junio C Hamano, git
In-Reply-To: <2AB285BD-DA6E-49D1-B664-B1A8B552DD76@midwinter.com>
Steven Grimm <koreth@midwinter.com> writes:
> Git's command line interface is already confusing enough without
> adding this kind of variability
It also makes portable scripts harder. It's easy to write a script
that works only if you have the option set ...
--
Matthieu
^ permalink raw reply
* Re: [PATCH] Make "git stash" configurable
From: Steven Grimm @ 2007-12-21 17:23 UTC (permalink / raw)
To: しらいしななこ
Cc: Junio C Hamano, git
In-Reply-To: <200712211026.lBLAQWWM014059@mi0.bluebottle.com>
On Dec 21, 2007, at 2:22 AM, しらいしななこ wrote:
> "git stash" without argument originally created an unnamed
> stash, but some people felt this can be confusing to new
> users. This introduces per-user config variable stash.quick to
> control this behavior.
This whole thing smells of indecision to me. We should figure out a
behavior we want and go with it. Git's command line interface is
already confusing enough without adding this kind of variability --
any new user is going to look at this and say, "Why couldn't they just
make up their minds?"
My vote is to require "save". We already have an alias mechanism that
people can use to turn "git stash save" into a two-word command. And I
say this as someone who runs "git stash" and will have to retrain his
fingers.
-Steve (who thinks "trying to please everyone at all times" is usually
a source of bad UI design decisions)
^ permalink raw reply
* Re: [PATCH v2] Make git send-email accept $EDITOR with arguments
From: Junio C Hamano @ 2007-12-21 17:02 UTC (permalink / raw)
To: Gustaf Hendeby; +Cc: luciano, git, gitster
In-Reply-To: <1198237002-21470-1-git-send-email-hendeby@isy.liu.se>
Gustaf Hendeby <hendeby@isy.liu.se> writes:
> Junio, even if this is technically not a bug fix, it would be nice to
> get this fix into the 1.5.4 so that the usage of $EDITOR becomes more
> consistent throughout git.
I can buy that, but at least a single line comment in front of that
system() explaining why this is safe to do so would be beneficial. I
suspect that somebody would propose moving $compse_filename inside
$GIT_DIR, now people realized $compose_filename is currently "./.msg.$$",
and $GIT_DIR could be anything. Quotemeta would probably be better as the
code you are touching won't be affected by a future change to the value of
that constant defined far away in the source.
^ permalink raw reply
* Re: Linux 2.6.24-rc6
From: Junio C Hamano @ 2007-12-21 16:56 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Kyle McMartin, Git Mailing List, Linux Kernel Mailing List
In-Reply-To: <alpine.LFD.0.9999.0712202054350.21557@woody.linux-foundation.org>
Linus Torvalds <torvalds@linux-foundation.org> writes:
> Actually, the code to finding one '\n' is still needed to avoid the
> (pathological) case of getting a "\No newline", so scrap that one which
> was too aggressive, and use this (simpler) one instead.
>
> Not that it matters in real life, since nobody uses -U0, and "git blame"
> won't care. But let's get it right anyway ;)
Actually "blame won't care" is a bit too strong. It's only we
(you) made it not to care. It is a different story if the
change to make it not to care was sensible.
The diff text "git blame" will see is affected with the tail
trimming optimization exactly the same way as the optimization
triggered this thread. With the common tails trimmed, the hunks
match differently from the way they match without trimming (the
gcc changelog testcase has differences between the unoptimized
blame and the tail-trimming one --- your original to put this
logic only in blame and my rewrite to move it in xdiff-interface
produce the same result that is different from the unoptimized
version, although both are 4x faster than the original).
When there are multiple possible matches, any match among them
is a correct match, and a match with a line in a blob is a match
to the blob no matter what line the match is anyway. The usual
workflow of checking blame to find the commit that introduced
the change and then going to "git show" to view the bigger
picture won't get affected. But the blamed line numbers will be
different from the unoptimized blame, and it may not match the
expectation of human readers. It won't match "git show" output
of the blamed commit.
> This whole function has had more bugs than it has lines.
I have to agree. It started as a brilliant idea but then it was
enhanced (in an attempt to support context) and executed not so
brilliantly.
^ permalink raw reply
* Re: [PATCH] git-pull: don't complain about branch merge config if only fetching tags
From: Junio C Hamano @ 2007-12-21 16:35 UTC (permalink / raw)
To: Gerrit Pape; +Cc: git
In-Reply-To: <20071221124400.20725.qmail@db93e79e204cd8.315fe32.mid.smarden.org>
Gerrit Pape <pape@smarden.org> writes:
> When running git pull with the -t switch, it properly fetches tags, but
> complains about missing information on how to merge. Since there's
> nothing to merge, make git-pull simply exit after fetching the tags.
>
> The problem has been reported by Joey Hess through
> http://bugs.debian.org/456035
>
> Signed-off-by: Gerrit Pape <pape@smarden.org>
> ---
> git-pull.sh | 5 +++++
> 1 files changed, 5 insertions(+), 0 deletions(-)
>
> diff --git a/git-pull.sh b/git-pull.sh
> index 698e82b..43be0bd 100755
> --- a/git-pull.sh
> +++ b/git-pull.sh
> @@ -112,6 +112,11 @@ case "$merge_head" in
> exit 1;;
> *) exit $?;;
> esac
> + # exit if only tags have been fetched
> + not_for_merge=$(sed -e '/ not-for-merge tag/d' \
> + "$GIT_DIR"/FETCH_HEAD)
> + test "$not_for_merge" != '' || exit 0
> +
> curr_branch=${curr_branch#refs/heads/}
>
> echo >&2 "You asked me to pull without telling me which branch you"
I've seen this patch on this list in the past, but isn't "git
pull -t" a user-error?
^ permalink raw reply
* Re: [PATCH] git-tag: fix -l switch handling regression.
From: Junio C Hamano @ 2007-12-21 16:32 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: git
In-Reply-To: <20071221105058.GD17701@artemis.madism.org>
Pierre Habouzit <madcoder@debian.org> writes:
> Signed-off-by: Pierre Habouzit <madcoder@debian.org>
> ---
>
> It seems that you didnt took that patch either, that IMHO gives a better
> semantics to git tag -l than yours, while keeping backward
> compatibility.
I thought this depended on some other changes. As is, doesn't
it break the t7004 test?
^ permalink raw reply
* [PATCH] git-describe: Add a --match option to limit considered tags.
From: Pierre Habouzit @ 2007-12-21 16:18 UTC (permalink / raw)
To: gitster; +Cc: git, Pierre Habouzit
Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
Documentation/git-describe.txt | 4 ++++
builtin-describe.c | 11 ++++++++---
2 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-describe.txt b/Documentation/git-describe.txt
index ac23e28..cb869e9 100644
--- a/Documentation/git-describe.txt
+++ b/Documentation/git-describe.txt
@@ -51,6 +51,10 @@ OPTIONS
being employed to standard error. The tag name will still
be printed to standard out.
+--match <pattern>::
+ Only consider tags matching the given pattern (can be used to avoid
+ leaking private tags made from the repository).
+
EXAMPLES
--------
diff --git a/builtin-describe.c b/builtin-describe.c
index 7a148a2..982a355 100644
--- a/builtin-describe.c
+++ b/builtin-describe.c
@@ -19,6 +19,7 @@ static int all; /* Default to annotated tags only */
static int tags; /* But allow any tags if --tags is specified */
static int abbrev = DEFAULT_ABBREV;
static int max_candidates = 10;
+const char *pattern = NULL;
struct commit_name {
int prio; /* annotated tag = 2, tag = 1, head = 0 */
@@ -57,9 +58,11 @@ static int get_name(const char *path, const unsigned char *sha1, int flag, void
* Otherwise only annotated tags are used.
*/
if (!prefixcmp(path, "refs/tags/")) {
- if (object->type == OBJ_TAG)
+ if (object->type == OBJ_TAG) {
prio = 2;
- else
+ if (pattern && fnmatch(pattern, path + 10, 0))
+ prio = 0;
+ } else
prio = 1;
}
else
@@ -253,7 +256,9 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
OPT_BOOLEAN(0, "tags", &tags, "use any tag in .git/refs/tags"),
OPT__ABBREV(&abbrev),
OPT_INTEGER(0, "candidates", &max_candidates,
- "consider <n> most recent tags (default: 10)"),
+ "consider <n> most recent tags (default: 10)"),
+ OPT_STRING(0, "match", &pattern, "pattern",
+ "only consider tags matching <pattern>"),
OPT_END(),
};
--
1.5.4.rc1.1096.gde4c4-dirty
^ permalink raw reply related
* [PATCH] Small comment fix for git-cvsimport.
From: Stefan Sperling @ 2007-12-21 15:57 UTC (permalink / raw)
To: git
Signed-off-by: Stefan Sperling <stsp@stsp.name>
---
git-cvsimport.perl | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/git-cvsimport.perl b/git-cvsimport.perl
index 92648f4..1fa9a22 100755
--- a/git-cvsimport.perl
+++ b/git-cvsimport.perl
@@ -848,7 +848,7 @@ while (<CVS>) {
}
if (!$opt_a && $starttime - 300 - (defined $opt_z ? $opt_z : 300) <= $date) {
# skip if the commit is too recent
- # that the cvsps default fuzz is 300s, we give ourselves another
+ # given that the cvsps default fuzz is 300s, we give ourselves another
# 300s just in case -- this also prevents skipping commits
# due to server clock drift
print "skip patchset $patchset: $date too recent\n" if $opt_v;
--
1.5.3.2
--
stefan
http://stsp.name PGP Key: 0xF59D25F0
^ permalink raw reply related
* Re: Problem with git-svn
From: Pascal Obry @ 2007-12-21 15:42 UTC (permalink / raw)
To: Pascal Obry; +Cc: Eric Wong, git list
In-Reply-To: <476AD1AB.8040406@gmail.com>
Pascal Obry a écrit :
> Yes part of it but before the creation of the /importfromcvs/trunk/ that
> was moved later as /trunk/PROJ.
I meant moved as /trunk/PROJ1 then /trunk/PROJ2... and so on.
Pascal.
--
--|------------------------------------------------------
--| Pascal Obry Team-Ada Member
--| 45, rue Gabriel Peri - 78114 Magny Les Hameaux FRANCE
--|------------------------------------------------------
--| http://www.obry.net
--| "The best way to travel is by means of imagination"
--|
--| gpg --keyserver wwwkeys.pgp.net --recv-key C1082595
^ permalink raw reply
* Re: [PATCH v2] Make git send-email accept $EDITOR with arguments
From: Gustaf Hendeby @ 2007-12-21 15:23 UTC (permalink / raw)
To: Jeff King; +Cc: luciano, git, gitster
In-Reply-To: <20071221133412.GA15198@sigill.intra.peff.net>
On 12/21/2007 02:34 PM, Jeff King wrote:
> On Fri, Dec 21, 2007 at 12:36:42PM +0100, Gustaf Hendeby wrote:
>
>> - system($editor, $compose_filename);
>> + system("$editor $compose_filename");
>
> If you are going to do it that way, I suspect you want to quotemeta
> $compose_filename.
>
> -Peff
Generally that would be true, but is that really necessary when I know
$compose_filename is defined as:
my $compose_filename = ".msg.$$";
Or, should I take it that you prefer the version using split? I didn't
really feel good about the possibility of splitting paths with spaces
that came with that one though.
/Gustaf
^ permalink raw reply
* Re: [PATCH v0] sha1_name: grok <revision>:./<relative-path>
From: Nguyen Thai Ngoc Duy @ 2007-12-21 14:17 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Dana How, Linus Torvalds, Alex Riesen, Jakub Narebski, git
In-Reply-To: <Pine.LNX.4.64.0712191334460.23902@racer.site>
On Dec 19, 2007 8:40 PM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
>
> When you are in a deeply-nested directory structure, and just want
> to reference a blob in a past revision, it can be pretty slow to
> type out "HEAD~29:/bla/blub/.../that-file".
>
> This patch makes "HEAD~29:./that-file" substitute the current prefix
> for "./". If there is not working directory, the prefix is empty.
>
> Note that this patch does not handle "../", and neither do I plan to.
Junio's rc1 announcement got me to read this. It would be indeed
useful as I usually work in deep subdirs. However, from my user
perspective, the right approach is to make <treeish>:path always be
relative to current directory. If you want absolute path, use
<treeish>:/path. More intuitive but it breaks current behavior. Can we
slowly migrate from current absolute-path-by-default behavior to
relative-pat- by-default one? (I don't know how to make such migration
smoothly though)
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>
> On Tue, 18 Dec 2007, Dana How wrote:
>
> > On Dec 18, 2007 5:16 PM, Linus Torvalds
> > <torvalds@linux-foundation.org> wrote:
> > > On Tue, 18 Dec 2007, Dana How wrote:
> > >
> > > > The cases we are talking about are all subtrees of the
> > > > working tree. There is a useful cwd suffix.
> > >
> > > No.
> > >
> > > The cases we're talking of are *not* subtrees of the working
> > > tree.
> > >
> > > The SHA1 of a commit may well be a totally disjoint tree. Try
> > > it in the git repository with something like
> >
> > Agreed, but note you wrote *may*.
>
> Okay, this is a proposed patch. It leaves the existing
> "HEAD:<path>" handling alone, and only touches "HEAD:./<path>",
> which would have been invalid anyway (except if you hacked your
> objects database to include a tree named ".").
>
> Note: this patch is not meant for application directly. It should
> be split into get_current_prefix() as one patch, and the
> sha1_name.c stuff as the second. (Not only to boost my ohloh
> statistics, but because they are logically two separate things.)
>
> Note, too: this is a quick and little-bit-dirty patch, not well
> tested. Particularly, I was unable to trigger the "No <path> in
> <rev>" error path, so I am not confident that this handling is
> correct.
>
> Note also: in contrast to Alex' approach, this will not only work
> for git-show, but for all callers of get_sha1().
>
> cache.h | 1 +
> setup.c | 16 +++++++++++++---
> sha1_name.c | 17 ++++++++++++++---
> 3 files changed, 28 insertions(+), 6 deletions(-)
>
> diff --git a/cache.h b/cache.h
> index 39331c2..83a2c31 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -225,6 +225,7 @@ extern char *get_index_file(void);
> extern char *get_graft_file(void);
> extern int set_git_dir(const char *path);
> extern const char *get_git_work_tree(void);
> +extern const char *get_current_prefix(void);
>
> #define ALTERNATE_DB_ENVIRONMENT "GIT_ALTERNATE_OBJECT_DIRECTORIES"
>
> diff --git a/setup.c b/setup.c
> index b59dbe7..fb9b680 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -3,6 +3,12 @@
>
> static int inside_git_dir = -1;
> static int inside_work_tree = -1;
> +static const char *current_prefix;
> +
> +const char *get_current_prefix()
> +{
> + return current_prefix;
> +}
>
> const char *prefix_path(const char *prefix, int len, const char *path)
> {
> @@ -267,6 +273,7 @@ const char *setup_git_directory_gently(int *nongit_ok)
> /* config may override worktree */
> if (check_repository_format_gently(nongit_ok))
> return NULL;
> + current_prefix = retval;
> return retval;
> }
> if (check_repository_format_gently(nongit_ok))
> @@ -279,7 +286,8 @@ const char *setup_git_directory_gently(int *nongit_ok)
> if (chdir(work_tree_env) < 0)
> die ("Could not chdir to %s", work_tree_env);
> strcat(buffer, "/");
> - return retval;
> + current_prefix = retval;
> + return current_prefix;
> }
> if (nongit_ok) {
> *nongit_ok = 1;
> @@ -339,7 +347,8 @@ const char *setup_git_directory_gently(int *nongit_ok)
> offset++;
> cwd[len++] = '/';
> cwd[len] = 0;
> - return cwd + offset;
> + current_prefix = cwd + offset;
> + return current_prefix;
> }
>
> int git_config_perm(const char *var, const char *value)
> @@ -396,7 +405,8 @@ const char *setup_git_directory(void)
> if (retval && chdir(retval))
> die ("Could not jump back into original cwd");
> rel = get_relative_cwd(buffer, PATH_MAX, get_git_work_tree());
> - return rel && *rel ? strcat(rel, "/") : NULL;
> + current_prefix = rel && *rel ? strcat(rel, "/") : NULL;
> + return current_prefix;
> }
>
> return retval;
> diff --git a/sha1_name.c b/sha1_name.c
> index 13e1164..6f61d26 100644
> --- a/sha1_name.c
> +++ b/sha1_name.c
> @@ -712,9 +712,20 @@ int get_sha1_with_mode(const char *name, unsigned char *sha1, unsigned *mode)
> }
> if (*cp == ':') {
> unsigned char tree_sha1[20];
> - if (!get_sha1_1(name, cp-name, tree_sha1))
> - return get_tree_entry(tree_sha1, cp+1, sha1,
> - mode);
> + if (!get_sha1_1(name, cp-name, tree_sha1)) {
> + const char *prefix;
> + if (!prefixcmp(cp + 1, "./") &&
> + (prefix = get_current_prefix())) {
> + unsigned char subtree_sha1[20];
> + if (get_tree_entry(tree_sha1, prefix,
> + subtree_sha1, mode))
> + return error("No '%s' in '%.*s'",
> + prefix, cp-name, name);
> + memcpy(tree_sha1, subtree_sha1, 20);
> + cp += 2;
> + }
> + return get_tree_entry(tree_sha1, cp+1, sha1, mode);
> + }
> }
> return ret;
> }
> --
> 1.5.4.rc0.72.g536e9
>
> -
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
Duy
^ permalink raw reply
* Re: [PATCH v2] Make git send-email accept $EDITOR with arguments
From: Jeff King @ 2007-12-21 13:34 UTC (permalink / raw)
To: Gustaf Hendeby; +Cc: luciano, git, gitster
In-Reply-To: <1198237002-21470-1-git-send-email-hendeby@isy.liu.se>
On Fri, Dec 21, 2007 at 12:36:42PM +0100, Gustaf Hendeby wrote:
> - system($editor, $compose_filename);
> + system("$editor $compose_filename");
If you are going to do it that way, I suspect you want to quotemeta
$compose_filename.
-Peff
^ permalink raw reply
* Re: Question about git-svn import
From: Pascal Obry @ 2007-12-21 13:11 UTC (permalink / raw)
To: Jörg Sommer; +Cc: git list
In-Reply-To: <20071220165227.GB22683@alea.gnuu.de>
Jörg Sommer a écrit :
> According to http://duncan.mac-vicar.com/blog/archives/282 it should help
> to do something like:
>
> git update-ref refs/remotes/git-svn master
> find -name .rev_db* | xargs rm
Ok, thanks a lot it is working!
Git is a great tool... and the community around it is really helpful.
Thanks to all!
Pascal.
--
--|------------------------------------------------------
--| Pascal Obry Team-Ada Member
--| 45, rue Gabriel Peri - 78114 Magny Les Hameaux FRANCE
--|------------------------------------------------------
--| http://www.obry.net
--| "The best way to travel is by means of imagination"
--|
--| gpg --keyserver wwwkeys.pgp.net --recv-key C1082595
^ permalink raw reply
* [PATCH] git-pull: don't complain about branch merge config if only fetching tags
From: Gerrit Pape @ 2007-12-21 12:44 UTC (permalink / raw)
To: git, Junio C Hamano
When running git pull with the -t switch, it properly fetches tags, but
complains about missing information on how to merge. Since there's
nothing to merge, make git-pull simply exit after fetching the tags.
The problem has been reported by Joey Hess through
http://bugs.debian.org/456035
Signed-off-by: Gerrit Pape <pape@smarden.org>
---
git-pull.sh | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/git-pull.sh b/git-pull.sh
index 698e82b..43be0bd 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -112,6 +112,11 @@ case "$merge_head" in
exit 1;;
*) exit $?;;
esac
+ # exit if only tags have been fetched
+ not_for_merge=$(sed -e '/ not-for-merge tag/d' \
+ "$GIT_DIR"/FETCH_HEAD)
+ test "$not_for_merge" != '' || exit 0
+
curr_branch=${curr_branch#refs/heads/}
echo >&2 "You asked me to pull without telling me which branch you"
--
1.5.3.7
^ permalink raw reply related
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