* git consistency and --help
From: Mike Frysinger @ 2007-12-21 9:37 UTC (permalink / raw)
To: git
along the lines of the different aspects of git being consistent,
shouldnt the shell scripts all be changed to call `man` ?
these call `man`:
git diff --help
git log --help
git stash --help
git rebase --help
git-diff --help
git-log --help
these show a short usage:
git-stash --help
git-rebase --help
looking at the code, it comes down to the git-sh-setup.sh file mostly ...
-mike
^ permalink raw reply
* Re: 1.5.4-rc2 plans
From: Mike Frysinger @ 2007-12-21 9:09 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Junio C Hamano, git
In-Reply-To: <476B6ABB.6040009@viscovery.net>
On Dec 21, 2007 2:26 AM, Johannes Sixt <j.sixt@viscovery.net> wrote:
> Junio C Hamano schrieb:
> > * Should "git stash" stop doing anything useful? I think the patch
> > from Nana today may be a reasonable compromise, although I still
> > think fundamentally different behaviour for the same command
> > configurable per-user is not very nice (we have precedent in "git
> > clean" already, though, but "git clean" is inherently dangerous
> > command, and "git stash" is much more useful and the issue impacts
> > more people).
>
> IMO we should give in and play the safe game. For those who don't like to
> type "git stash save" can always
>
> git config --global alias.shelve "stash save"
> git config --global alias.unshelve "stash apply"
>
> and retrain the fingers.
in the past, i used git merely to checkout code and send diffs to
maintainers ... never for my own work. ive started to transition from
using svn everywhere to trying out git, and saw reference to this
"stash" command on another list. i wanted to learn more about it, so
i started off with `git-stash` to get some info, and wondered what
just happened. then i typoed the --help option and wondered even more
what just happened :).
after flipping through the git mailing list for a while, it's good to
see that `git stash <random crap>` will be fixed in the next release,
and yes the default behavior of saving is confusing. the argument
that newbies can easily recover their work really only works if the
newbie knows what's going on. if they knew from the start, then they
wouldnt be newbies eh.
making the default behavior non-destructive (which is to say, not
changing anything) and allowing people to arbitrarily configure the
default behavior sounds sane to me. taking it up a level, people
could just as easily write functions in their shell environment to do
the same thing. which is to say that imho, the argument against this
for fear of different behavior depending on user is over the top.
configuration options are there to change the behavior based on the
user's preference.
-mike
^ permalink raw reply
* Re: 1.5.4-rc2 plans
From: Steven Grimm @ 2007-12-21 8:57 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vwsr8lwf7.fsf@gitster.siamese.dyndns.org>
On Dec 20, 2007, at 4:32 PM, Junio C Hamano wrote:
> * handling of EDITOR in git commit and git tag is currently different.
> It expects "vi" not "vi --some-funny-option". I sent out a
> for-discussion patch after seeing Steven's and Luciano's.
I'm perfectly fine with your approach (punt and let the shell figure
it out). It's certainly a lot less code change than mine. However, if
you want another rev of mine with Johannes' comments taken into
account, I should be able to do that in the next day or two.
-Steve
^ permalink raw reply
* Re: git-stash: RFC: Adopt the default behavior to other commands
From: しらいしななこ @ 2007-12-21 8:40 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jörg Sommer
In-Reply-To: <7v4pedov6c.fsf@gitster.siamese.dyndns.org>
Quoting Junio C Hamano <gitster@pobox.com>:
> しらいしななこ <nanako3@bluebottle.com> writes:
>
>> How about making this behavior configurable?
>
> First, as a general principle, I'd like to avoid having commands that
> changes their behaviour drastically depending on who the user is. It
> makes it harder for people experienced a bit more than totally new to
> help others. If they are truly experts and are familiar about the
> configuration stash.quick, then they will be fine, but others would say
> "Well, it works for me -- 'git stash' itself won't stash but list. Why
> isn't it working for you, I don't know" and scratch head.
I see. I usually am not the person who helps but am the
person who is helped in such a situation, and did not
consider this issue.
> Having said that, I reserve rights to change my mind later and start
> liking this approach as a compromise.
I will change the patch as you suggested and resubmit. Let's
see if I can change your mind (^_^).
--
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/
----------------------------------------------------------------------
Free pop3 email with a spam filter.
http://www.bluebottle.com/tag/5
^ permalink raw reply
* Re: git-stash: RFC: Adopt the default behavior to other commands
From: Wincent Colaiuta @ 2007-12-21 7:59 UTC (permalink / raw)
To: Junio C Hamano
Cc: しらいしななこ, git,
Jörg Sommer
In-Reply-To: <7v4pedov6c.fsf@gitster.siamese.dyndns.org>
El 20/12/2007, a las 23:31, Junio C Hamano escribió:
> しらいしななこ <nanako3@bluebottle.com> writes:
>
>> How about making this behavior configurable?
>
> First, as a general principle, I'd like to avoid having commands that
> changes their behaviour drastically depending on who the user is. It
> makes it harder for people experienced a bit more than totally new to
> help others. If they are truly experts and are familiar about the
> configuration stash.quick, then they will be fine, but others would
> say
> "Well, it works for me -- 'git stash' itself won't stash but list.
> Why
> isn't it working for you, I don't know" and scratch head.
Although the patch is well-intentioned, I totally agree with Junio on
this point.
The solution here isn't configuration, but education. Some people have
already told how they've been burnt by doing an accidental stash, but
how many times does this have to happen to you before you learn your
lesson? Once? Twice if you are very unlucky?
And this is not a very painful lesson to learn, seeing as "git stash"
is not an inherently destructive operation. In 99% of cases there is
no risk of hard-to-reverse "damage". If you accidentally stash, you
can just unstash with "git stash apply". I already posted a two-line
patch which tells the user how to do this:
http://marc.info/?l=git&m=119799257404542&w=2
The 1% of cases in which "git stash apply" won't work is where the
user has unsaved changes in running editors at the time they do the
accidental stash. IMO, this is no justification to change the
behaviour of stash. Exactly the same is true of other commands that
alter the working tree; for example, what happens if you use "git
checkout" to switch to another branch when you have unsaved changes in
running editors? Are people suggesting that we should change the
behaviour of "git checkout" to warn the user that they should save any
unsaved changes before continuing and then hit "y"? I think that such
a thing would be absurd.
Cheers,
Wincent
^ permalink raw reply
* Re: 1.5.4-rc2 plans
From: Johannes Sixt @ 2007-12-21 7:26 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vwsr8lwf7.fsf@gitster.siamese.dyndns.org>
Junio C Hamano schrieb:
> * Should "git stash" stop doing anything useful? I think the patch
> from Nana today may be a reasonable compromise, although I still
> think fundamentally different behaviour for the same command
> configurable per-user is not very nice (we have precedent in "git
> clean" already, though, but "git clean" is inherently dangerous
> command, and "git stash" is much more useful and the issue impacts
> more people).
IMO we should give in and play the safe game. For those who don't like to
type "git stash save" can always
git config --global alias.shelve "stash save"
git config --global alias.unshelve "stash apply"
and retrain the fingers.
-- Hannes
^ permalink raw reply
* [PATCH] git-send-email: Add --suppress-all-from option.
From: David Brown @ 2007-12-21 6:01 UTC (permalink / raw)
To: git; +Cc: David Brown
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. An
common example would be an internal discussion at a company to ask if
a particular patch should be applied. Some environments may even have
policy against such mail being sent outside of the company.
Add the --suppress-all-from/--no-suppress-all-from options to avoid
sending patches to the patch author, even if different from the
sender. Add the sendemail.suppressallfrom config option to allow this
to have a different default.
Signed-off-by: David Brown <git@davidb.org>
---
Documentation/git-send-email.txt | 7 +++++++
git-send-email.perl | 9 ++++++++-
2 files changed, 15 insertions(+), 1 deletions(-)
diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index f0bd285..5d06264 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -117,6 +117,13 @@ The --cc option must be repeated for each user you want on the cc list.
Default is the value of 'sendemail.suppressfrom' configuration value;
if that is unspecified, default to --no-suppress-from.
+--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.
+
--thread, --no-thread::
If this is set, the In-Reply-To header will be set on each email sent.
If disabled with "--no-thread", no emails will have the In-Reply-To
diff --git a/git-send-email.perl b/git-send-email.perl
index 248d035..80265b5 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -90,6 +90,9 @@ Options:
--suppress-from Suppress sending emails to yourself. Defaults to off.
+ --suppress-all-from Never automatically send to a patch author.
+ Defaults to off.
+
--thread Specify that the "In-Reply-To:" header should be set on all
emails. Defaults to on.
@@ -174,7 +177,8 @@ if ($@) {
my ($quiet, $dry_run) = (0, 0);
# Variables with corresponding config settings
-my ($thread, $chain_reply_to, $suppress_from, $signed_off_cc, $cc_cmd);
+my ($thread, $chain_reply_to, $suppress_from, $suppress_all_from);
+my ($signed_off_cc, $cc_cmd);
my ($smtp_server, $smtp_server_port, $smtp_authuser, $smtp_authpass, $smtp_ssl);
my ($identity, $aliasfiletype, @alias_files, @smtp_host_parts);
@@ -182,6 +186,7 @@ my %config_bool_settings = (
"thread" => [\$thread, 1],
"chainreplyto" => [\$chain_reply_to, 1],
"suppressfrom" => [\$suppress_from, 0],
+ "suppressallfrom" => [\$suppress_all_from, 0],
"signedoffcc" => [\$signed_off_cc, 1],
"smtpssl" => [\$smtp_ssl, 0],
);
@@ -218,6 +223,7 @@ my $rc = GetOptions("sender|from=s" => \$sender,
"quiet" => \$quiet,
"cc-cmd=s" => \$cc_cmd,
"suppress-from!" => \$suppress_from,
+ "suppress-all-from!" => \$suppress_all_from,
"signed-off-cc|signed-off-by-cc!" => \$signed_off_cc,
"dry-run" => \$dry_run,
"envelope-sender=s" => \$envelope_sender,
@@ -700,6 +706,7 @@ foreach my $t (@files) {
$subject = $1;
} elsif (/^(Cc|From):\s+(.*)$/) {
+ next if ($suppress_all_from);
if (unquote_rfc2047($2) eq $sender) {
next if ($suppress_from);
}
--
1.5.3.7
^ permalink raw reply related
* Re: Linux 2.6.24-rc6
From: Linus Torvalds @ 2007-12-21 5:21 UTC (permalink / raw)
To: Junio C Hamano, Git Mailing List
In-Reply-To: <alpine.LFD.0.9999.0712202054350.21557@woody.linux-foundation.org>
On Thu, 20 Dec 2007, Linus Torvalds wrote:
>
> Not that it matters in real life, since nobody uses -U0, and "git blame"
> won't care. But let's get it right anyway ;)
.. and here's a test-case to see the *impact* of this whole issue:
yes | head -1000 > a
yes | head -2000 > b
git diff -U0 a b | grep @@
diff -U0 a b | grep @@
and notice the differences.
Both will add a thousand lines of "y", but the "git diff" will add it at a
different point, ie git says:
@@ -488,0 +489,1000 @@ y
while a non-tail-optimizing diff will say
@@ -1000,0 +1001,1000 @@
which may be a bit more obvious.
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.
(We could also go to exactly where the differences started, and if the
previous character was a newline or the beginning of the file, we'd not
move forward at all, and then we'd get a more "logical" diff of inserting
at the beginning).
Considering that the answer is correct, and this only happens for insane
cases anyway, I don't really think anybody cares, but it's an interesting
case nonetheless.
Linus
^ permalink raw reply
* Re: Linux 2.6.24-rc6
From: Linus Torvalds @ 2007-12-21 4:58 UTC (permalink / raw)
To: Kyle McMartin, Junio C Hamano, Git Mailing List; +Cc: Linux Kernel Mailing List
In-Reply-To: <alpine.LFD.0.9999.0712202009290.21557@woody.linux-foundation.org>
On Thu, 20 Dec 2007, Linus Torvalds wrote:
>
> And here's the git patch to avoid this optimization when there is
> context.
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 ;)
This whole function has had more bugs than it has lines.
Linus
---
xdiff-interface.c | 7 +++++--
1 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/xdiff-interface.c b/xdiff-interface.c
index 9ee877c..711029e 100644
--- a/xdiff-interface.c
+++ b/xdiff-interface.c
@@ -115,15 +115,18 @@ static void trim_common_tail(mmfile_t *a, mmfile_t *b, long ctx)
char *bp = b->ptr + b->size;
long smaller = (a->size < b->size) ? a->size : b->size;
+ if (ctx)
+ return;
+
while (blk + trimmed <= smaller && !memcmp(ap - blk, bp - blk, blk)) {
trimmed += blk;
ap -= blk;
bp -= blk;
}
- while (recovered < trimmed && 0 <= ctx)
+ while (recovered < trimmed)
if (ap[recovered++] == '\n')
- ctx--;
+ break;
a->size -= (trimmed - recovered);
b->size -= (trimmed - recovered);
}
^ permalink raw reply related
* Re: Linux 2.6.24-rc6
From: Kyle McMartin @ 2007-12-21 4:50 UTC (permalink / raw)
To: Linus Torvalds
Cc: Junio C Hamano, Git Mailing List, Raphael Assenat, Andrew Morton,
Linux Kernel Mailing List
In-Reply-To: <alpine.LFD.0.9999.0712202026040.21557@woody.linux-foundation.org>
On Thu, Dec 20, 2007 at 08:40:54PM -0800, Linus Torvalds wrote:
> That was a rather long-winded explanation of what happened, mainly because
> it was all very unexpected to me, and I had personally mistakenly thought
> the git optimization was perfectly valid and actually had to go through
> the end result to see what was going on.
>
> Anyway, the diff on kernel.org should be all ok now, and mirrored out too.
>
Thanks again for being so quick to track this down, applies fine and is
out for building in rawhide now.
cheers, Kyle
^ permalink raw reply
* Re: Linux 2.6.24-rc6
From: Linus Torvalds @ 2007-12-21 4:40 UTC (permalink / raw)
To: Kyle McMartin, Junio C Hamano, Git Mailing List, Raphael Assenat,
Andrew Morton
Cc: Linux Kernel Mailing List
In-Reply-To: <alpine.LFD.0.9999.0712202009290.21557@woody.linux-foundation.org>
On Thu, 20 Dec 2007, Linus Torvalds wrote:
>
> It only happened for a few files that had lots of repeated lines - so that
> the diff could literally be done multiple different ways - and in fact,
> the file that caused the problems really had a bogus commit that
> duplicated *way* too much data, and caused lots of #define's to exist
> twice.
Here's the example of this kind of behaviour: in the 2.6.26-rc5 tree the
file drivers/video/mbx/reg_bits.h has the #defines for
/* DINTRS - Display Interrupt Status Register */
/* DINTRE - Display Interrupt Enable Register */
duplicated twice due to commit ba282daa919f89c871780f344a71e5403a70b634
("mbxfb: Improvements and new features") by Raphael Assenat mistakenly
adding another copy of the same old set of defines that we already got
added once before by commit fb137d5b7f2301f2717944322bba38039083c431
("mbxfb: Add more registers bits access macros").
Now, that was a mistake - and one that probably happened because Rafael or
more likely Andrew Morton used GNU patch with its insane defaults (which
is to happily apply the same patch that adds things twice, because it
doesn't really care if the context matches or not).
But what that kind of thing causes is that when you create a patch of the
end result, it can show the now new duplicate lines two different (but
equally valid) ways: it can show it as an addition of the _first_ set of
lines, or it can show it as an addition of the _second_ set of lines. They
are the same, after all.
Now, it doesn't really matter which way you choose to show it, although
because of how "git diff" finds similarities, it tends to prefer to show
the second set of identical lines as the "new" ones. Which is generally
reasonable.
However, that interacted really badly with the new git logic that said
that "if the two files end in the same sequence, just ignore the common
tail of the file", because the latter copy of the identical lines would
now show up as _part_ of that common tail, so the lines that the git diff
machinery would normally like to show up as "new" did in fact end up being
considered uninteresting, because they were part of an idential tail.
So now "git diff" would happily pick _earlier_ lines as the new ones, and
it would still be a conceptually valid diff, but because we had trimmed
the tail of the file, that conceptually valid diff no longer had the
expected shared context at the end.
And while it's a bit embarrassing, I'm really rather happy that both GNU
patch and "git apply" actually refused to apply the patch. It may have
been "conceptually correct" (ie it did really contain all of the changes!)
but because it lacked the expected context it really wasn't a good patch.
That was a rather long-winded explanation of what happened, mainly because
it was all very unexpected to me, and I had personally mistakenly thought
the git optimization was perfectly valid and actually had to go through
the end result to see what was going on.
Anyway, the diff on kernel.org should be all ok now, and mirrored out too.
Linus
^ permalink raw reply
* Re: Linux 2.6.24-rc6
From: Linus Torvalds @ 2007-12-21 4:22 UTC (permalink / raw)
To: Kyle McMartin, Junio C Hamano, Git Mailing List; +Cc: Linux Kernel Mailing List
In-Reply-To: <alpine.LFD.0.9999.0712201937470.21557@woody.linux-foundation.org>
On Thu, 20 Dec 2007, Linus Torvalds wrote:
>
> The tar-ball and the git archive itself is fine, but yes, the diff from
> 2.6.23 to 2.6.24-rc6 is bad. It's the "trim_common_tail()" optimization
> that has caused way too much pain.
Very interesting breakage. The patch was actually "correct" in a (rather
limited) technical sense, but the context at the end was missing because
while the trim_common_tail() code made sure to keep enough common context
to allow a valid diff to be generated, the diff machinery itself could
decide that it could generate the diff differently than the "obvious"
solution.
It only happened for a few files that had lots of repeated lines - so that
the diff could literally be done multiple different ways - and in fact,
the file that caused the problems really had a bogus commit that
duplicated *way* too much data, and caused lots of #define's to exist
twice.
But the sad fact appears that the git optimization (which is very
important for "git blame", which needs no context), is only really valid
for that one case where we really don't need any context.
I uploaded a fixed patch. And here's the git patch to avoid this
optimization when there is context.
Linus
---
xdiff-interface.c | 12 ++++++------
1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/xdiff-interface.c b/xdiff-interface.c
index 9ee877c..0b7e057 100644
--- a/xdiff-interface.c
+++ b/xdiff-interface.c
@@ -110,22 +110,22 @@ 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;
- long trimmed = 0, recovered = 0;
+ long trimmed = 0;
char *ap = a->ptr + a->size;
char *bp = b->ptr + b->size;
long smaller = (a->size < b->size) ? a->size : b->size;
+ if (ctx)
+ return;
+
while (blk + trimmed <= smaller && !memcmp(ap - blk, bp - blk, blk)) {
trimmed += blk;
ap -= blk;
bp -= blk;
}
- while (recovered < trimmed && 0 <= ctx)
- if (ap[recovered++] == '\n')
- ctx--;
- a->size -= (trimmed - recovered);
- b->size -= (trimmed - recovered);
+ a->size -= trimmed;
+ b->size -= trimmed;
}
int xdi_diff(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp, xdemitconf_t const *xecfg, xdemitcb_t *xecb)
^ permalink raw reply related
* Re: Linux 2.6.24-rc6
From: Kyle McMartin @ 2007-12-21 3:50 UTC (permalink / raw)
To: Linus Torvalds
Cc: Junio C Hamano, Git Mailing List, Linux Kernel Mailing List
In-Reply-To: <alpine.LFD.0.9999.0712201937470.21557@woody.linux-foundation.org>
On Thu, Dec 20, 2007 at 07:49:05PM -0800, Linus Torvalds wrote:
>
>
> On Thu, 20 Dec 2007, Kyle McMartin wrote:
> >
> > I think I see the problem, it's lack of context in the diff,
>
> No, the problem is that "git diff" is apparently broken by a recent
> optimization. The diff is simply broken.
>
> The tar-ball and the git archive itself is fine, but yes, the diff from
> 2.6.23 to 2.6.24-rc6 is bad. It's the "trim_common_tail()" optimization
> that has caused way too much pain.
>
> Sorry about that, I'll fix it up asap.
>
no biggie, thanks!
cheers, Kyle
^ permalink raw reply
* Re: Linux 2.6.24-rc6
From: Linus Torvalds @ 2007-12-21 3:49 UTC (permalink / raw)
To: Kyle McMartin, Junio C Hamano, Git Mailing List; +Cc: Linux Kernel Mailing List
In-Reply-To: <20071221030152.GC8535@fattire.cabal.ca>
On Thu, 20 Dec 2007, Kyle McMartin wrote:
>
> I think I see the problem, it's lack of context in the diff,
No, the problem is that "git diff" is apparently broken by a recent
optimization. The diff is simply broken.
The tar-ball and the git archive itself is fine, but yes, the diff from
2.6.23 to 2.6.24-rc6 is bad. It's the "trim_common_tail()" optimization
that has caused way too much pain.
Sorry about that, I'll fix it up asap.
Linus
^ permalink raw reply
* Re: [PATCH] git-commit: add --verbatim to allow unstripped commit messages
From: Linus Torvalds @ 2007-12-21 0:49 UTC (permalink / raw)
To: Bj?rn Steinbrink; +Cc: Alex Riesen, git
In-Reply-To: <20071221001449.GA10607@atjola.homenet>
On Fri, 21 Dec 2007, Bj?rn Steinbrink wrote:
>
> Hm, this is a bit more intrusive, but should catch most cases.
>
> At the top of the comments in the commit message template add:
> #GIT CUT HERE
> (And adjust the descriptive text)
Ouch. I'd personally hate to see something like that at the top and then
have to save it. I always add my text to the top of the message, and all
the pre-made messages for me are at the top (ie the kinds you get with
"git commit --amend", where the top of the thing is the old message).
That said, I might well agree with this approach if we made the marker
line be the *last* line of the message, ie make it be something that ends
with (ignoring empty whitespace at the end, of course, since those are
invisible in most editors):
# Remove this line to keep all comment lines
or something like that.
That still keeps the question about whitespace cleanups. But if it's just
about whitespace or no whitespace, then a simple "--verbatim" flag would
work.
Linus
^ permalink raw reply
* Re: [PATCH v2] builtin-tag.c: allow arguments in $EDITOR
From: Steven Grimm @ 2007-12-21 0:33 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, Luciano Rocha, git
In-Reply-To: <7vhcidovxt.fsf@gitster.siamese.dyndns.org>
On Dec 20, 2007, at 2:14 PM, Junio C Hamano wrote:
> I think Steven stopped after you poked holes in that patch.
Nah, just entered a particularly busy period in my day job and haven't
had time to do much more git stuff than occasionally skim the mailing
list. I do plan to revisit that at some point unless the patch in your
mail ends up being what we go with. (It seems like a sensible approach
to me.)
-Steve
^ permalink raw reply
* 1.5.4-rc2 plans
From: Junio C Hamano @ 2007-12-21 0:32 UTC (permalink / raw)
To: git
I've tagged -rc1 last night. The changes are mostly fixes. There are
some remaining issues I'd like to see fixed/decided before 1.5.4.
One important issue is to identify and fix regressions since 1.5.3
series. No "rewrite scripted git-foo completely in C" can be regression
free, and we had quite a few internal changes during 1.5.4 cycle (not
just rewrite to C, but C level uses new and improved API such as strbuf
and parse-options). Currently I am aware of these regressions:
* handling of options, "--abbrev 10 HEAD", "--abbrev=10 HEAD" and
"--abbrev HEAD". The last one does not work for commands that use
parse-options. Pierre is on top of this, I hope.
* handling of EDITOR in git commit and git tag is currently different.
It expects "vi" not "vi --some-funny-option". I sent out a
for-discussion patch after seeing Steven's and Luciano's.
but I am sure there must be others that we haven't even identified.
Also there have been handful usability issues, which can be solved
without incompatible changes:
* Should "git stash" stop doing anything useful? I think the patch
from Nana today may be a reasonable compromise, although I still
think fundamentally different behaviour for the same command
configurable per-user is not very nice (we have precedent in "git
clean" already, though, but "git clean" is inherently dangerous
command, and "git stash" is much more useful and the issue impacts
more people).
* Introduction of "<tree>:./path" (Dscho). I could be talked into
accepting the patch if it is useful to people who live deep within
subdirectories.
* Making commit log message cleansing optionally less aggressive. I do
not think we have seen the end of the thread yet, but I think Linus's
"three cleansing levels" approach is on the right track.
I'd like to see the above resolved and hopefully 12 other regressions
identified and fixed before the end of the year, when -rc2 can hopefully
happen.
The following technical issues have been raised but not resolved and I
do not expect the resolution to be part of 1.5.4.
* Authenticated pserver (Ævar Arnfjörð Bjarmason). It needs security
audit of the code and also the password storage needs to be decided.
This can wait post 1.5.4.
* Threaded "repack -a -d -f" when having a verify tight pack suffers
from massive malloc(3) memory fragmentation, which we cannot do much
about.
* Rebase using "format-patch | am" machinery has issues when dealing
with a mostly-text change that includes NUL (or anything you cannot
e-mail patch for). The workaround is "rebase -m" which unfortunatly
is slow. I am working on rewriting cherry-pick whenever I find time
to address it, though.
* Handling of trailing blank lines does not mesh well with the way diff
and apply whitespace logic is done.
^ permalink raw reply
* Re: [PATCH] git-commit: add --verbatim to allow unstripped commit messages
From: Björn Steinbrink @ 2007-12-21 0:14 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Alex Riesen, git
In-Reply-To: <alpine.LFD.0.9999.0712201545450.21557@woody.linux-foundation.org>
On 2007.12.20 15:55:18 -0800, Linus Torvalds wrote:
>
>
> On Fri, 21 Dec 2007, Alex Riesen wrote:
> >
> > Yes, I afraid I need both. I use "git commit -t" almost (submission in
> > perforce takes careful planning) every day. I also would like to keep
> > the empty leading and trailing lines (perforce default GUI P4Win does
> > not show them, but our scripts which check the descriptions will test
> > the description text according to template which does have trailing
> > empty lines).
>
> Hmm. I think your updated patch was pretty good, although I still think it
> could be improved a bit. In particular, thinking more about it, I think we
> have more than an "on/off" switch - we really have three cases:
>
> a) strip whitespace _and_ comments
> b) strip unnecessary whitespace only
> c) leave things _totally_ alone
>
> and on top of that we also have the issue of an editor.
>
> So my patch basically said that in the absense of an editor, we'll still
> clean up whitespace, but not comments (ie "no_edit" implies doing (b)
> rather than (b)), while your patch basically results in (c) regardless of
> whether we run an editor or not.
>
> But that still leaves one case: do we ever want to do (b) even *if* we use
> an editor? There's another possible choice: our old behaviour of (a) in
> the presense of an editor is now gone.
>
> Now, that last choice (ie "case (a) without an editor") is not only
> unlikely to be anything people want to do anyway, it's also easy enough to
> do by just using "git stripspace -s" on whatever non-editor thing you feed
> to "git commit", so I don't think we need to worry about that one.
>
> But the "maybe you want to run an editor, and you _do_ want case (b)"
> sounds like a case that is not at all unlikely. I could easily see the
> case where you want to have a template that uses '#', and *despite* that
> you want to (a) allow the user to edit things _and_ (b) clean up
> whitespace too.
>
> So I'd almost suggest you make the "--verbatim" flag a three-way switch,
> to allow "totally verbatim" (leave everything in place) and a "don't touch
> comments" (just fix up whitespace) mode.
>
> Hmm? Does that make sense to you?
Hm, this is a bit more intrusive, but should catch most cases.
At the top of the comments in the commit message template add:
#GIT CUT HERE
(And adjust the descriptive text)
That line hopefully being uncommon enough to not affect any existing
stuff.
If that line is present, comment lines above it are kept, otherwise
they're removed. Whitespace is always fixed(?).
Results:
- git commit -m "# Foo and bar"
* keeps the comment, looks like the expected thing
- git commit with editor
* Comments that are manually added are kept
* For the (probably seldom) case, that you want manually added
comments to be stripped, you can still remove the "#GIT CUR HERE"
line
- git commit with template
* The existing templates probably won't have a "#GIT CUT HERE" line,
so we're backwards compatible
* Templates that want to keep the comments can simple get a "#GIT CUT
HERE" line at the end and Just Work, regardless of whether or not
you forget to pass --verbatim.
Hmm?
thanks,
Björn
^ permalink raw reply
* Re: [PATCH] git-commit: add --verbatim to allow unstripped commit messages
From: Linus Torvalds @ 2007-12-21 0:03 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Alex Riesen, git
In-Reply-To: <7vlk7plydv.fsf@gitster.siamese.dyndns.org>
On Thu, 20 Dec 2007, Junio C Hamano wrote:
>
> But I wonder if people are using a workflow like this:
>
> $ cp $company_template my_message
> $ edit my_message
> $ git commit -F my_message
Yes. See the email I just sent to Alex.
I think that the above kind of workflow is trivial to support with the
"new" semantics (ie the patch under discussion), by just rewriting that
last step as
$ git stripspace -s < my_message | git-commit -F -
so in that sense, we don't really "lose" anything, although I do agree
that backwards compatibility (in case somebody actually does this!)
suffers.
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.
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.
Linus
^ permalink raw reply
* Re: [PATCH] git-commit: add --verbatim to allow unstripped commit messages
From: Linus Torvalds @ 2007-12-20 23:55 UTC (permalink / raw)
To: Alex Riesen; +Cc: git
In-Reply-To: <20071220233324.GB3052@steel.home>
On Fri, 21 Dec 2007, Alex Riesen wrote:
>
> Yes, I afraid I need both. I use "git commit -t" almost (submission in
> perforce takes careful planning) every day. I also would like to keep
> the empty leading and trailing lines (perforce default GUI P4Win does
> not show them, but our scripts which check the descriptions will test
> the description text according to template which does have trailing
> empty lines).
Hmm. I think your updated patch was pretty good, although I still think it
could be improved a bit. In particular, thinking more about it, I think we
have more than an "on/off" switch - we really have three cases:
a) strip whitespace _and_ comments
b) strip unnecessary whitespace only
c) leave things _totally_ alone
and on top of that we also have the issue of an editor.
So my patch basically said that in the absense of an editor, we'll still
clean up whitespace, but not comments (ie "no_edit" implies doing (b)
rather than (b)), while your patch basically results in (c) regardless of
whether we run an editor or not.
But that still leaves one case: do we ever want to do (b) even *if* we use
an editor? There's another possible choice: our old behaviour of (a) in
the presense of an editor is now gone.
Now, that last choice (ie "case (a) without an editor") is not only
unlikely to be anything people want to do anyway, it's also easy enough to
do by just using "git stripspace -s" on whatever non-editor thing you feed
to "git commit", so I don't think we need to worry about that one.
But the "maybe you want to run an editor, and you _do_ want case (b)"
sounds like a case that is not at all unlikely. I could easily see the
case where you want to have a template that uses '#', and *despite* that
you want to (a) allow the user to edit things _and_ (b) clean up
whitespace too.
So I'd almost suggest you make the "--verbatim" flag a three-way switch,
to allow "totally verbatim" (leave everything in place) and a "don't touch
comments" (just fix up whitespace) mode.
Hmm? Does that make sense to you?
Linus
^ permalink raw reply
* Re: [PATCH] git-commit: add --verbatim to allow unstripped commit messages
From: Junio C Hamano @ 2007-12-20 23:50 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Alex Riesen, git
In-Reply-To: <alpine.LFD.0.9999.0712201324270.21557@woody.linux-foundation.org>
I said in the log message when applying your version:
We even stripped "#" lines from messages given this way:
git commit -m "# Message starting with a hash-mark"
which was argurably a bug.
But I wonder if people are using a workflow like this:
$ cp $company_template my_message
$ edit my_message
$ git commit -F my_message
To them, this change is a regression, as $company_template may have had
insn like "# Here you describe your changes..." and the workflow relied
on the current behaviour of stripping them.
^ permalink raw reply
* [PATCH] Fix thinko in checking for commit message emptiness
From: Alex Riesen @ 2007-12-20 23:48 UTC (permalink / raw)
To: Linus Torvalds; +Cc: git, Junio C Hamano
In-Reply-To: <20071220233539.GC3052@steel.home>
Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
---
Sorry. It must be late. It it must have been late yesterday
and still is today.
builtin-commit.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/builtin-commit.c b/builtin-commit.c
index 1356d20..eae7661 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -436,7 +436,7 @@ static int message_is_empty(struct strbuf *sb, int start)
int eol, i;
if (verbatim_message && sb->len)
- return 1;
+ return 0;
/* See if the template is just a prefix of the message. */
strbuf_init(&tmpl, 0);
--
1.5.4.rc1.33.gbd32b
^ permalink raw reply related
* Re: [PATCH] Only filter "#" comments from commits if the editor is used
From: Alex Riesen @ 2007-12-20 23:43 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Linus Torvalds, git
In-Reply-To: <7vtzmdlyva.fsf@gitster.siamese.dyndns.org>
Junio C Hamano, Fri, Dec 21, 2007 00:39:37 +0100:
> Thanks.
Please hold on. The test fails.
^ permalink raw reply
* Re: [PATCH] Only filter "#" comments from commits if the editor is used
From: Junio C Hamano @ 2007-12-20 23:43 UTC (permalink / raw)
To: Alex Riesen; +Cc: Linus Torvalds, git
In-Reply-To: <20071220233714.GD3052@steel.home>
I said thanks a bit too early. That is on top of verbatim, which I have
doubts about putting in 1.5.4. I on the other hand think Linus's is a
pure bugfix.
Will tweak and apply, thanks.
^ permalink raw reply
* Re: [PATCH] Only filter "#" comments from commits if the editor is used
From: Junio C Hamano @ 2007-12-20 23:39 UTC (permalink / raw)
To: Alex Riesen; +Cc: Linus Torvalds, git
In-Reply-To: <20071220233714.GD3052@steel.home>
Thanks.
^ 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