* Re: [PATCH] Changed timestamp behavior of options -c/-C/--amend
From: Erick Mattos @ 2009-10-30 23:12 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v4opgh5qr.fsf@alter.siamese.dyndns.org>
2009/10/30 Junio C Hamano <gitster@pobox.com>:
> Erick Mattos <erick.mattos@gmail.com> writes:
>
>> 2009/10/30 Junio C Hamano <gitster@pobox.com>:
>>> Junio C Hamano <gitster@pobox.com> writes:
>>>
>>>> ...
>>>> I agree that the issue the patch addresses is worth improving, and I think
>>>> it is sensible to default to reuse the timestamp for -C and not to reuse
>>>> for --amend. I am not sure about -c myself, but it probably shouldn't
>>>> reuse the timestamp by default.
>>>
>>> So after realizing that this was about "author" timestamp, I am rescinding
>>> this comment about the change of the default for -c and --amend.
>>
>> Actually I am only changing the default for -c and I see it useful.
>> At least with me I normally use -c only to use messages of commits as
>> template.
>
> I do that from time to time as well. As I said in a different message, it
> may make the default more intutitive if we give new timestamp when the
> author is the same as the committer when doing "-c". You are creating
> your own commit in that case.
>
I don't see a use for comparing the author and committer because I can
use as template my own commits or others'.
Let's clarify the subject:
In my point-of-view -c option is mainly used for templating commit messages.
In that case -c has a different default from -C and --amend options
thus creating a need for two new options: --reuse-timestamp and
--no-reuse-timestamp.
As I see by your messages you do prefer to have all those options set
up for reusing timestamp as default.
In that case we just need one new option: --no-reuse-timestamp (or
--recreate-timestamp or whatever).
So now It is a matter of decision only and you are the guy.
What should be for all?
^ permalink raw reply
* Re: [PATCH 8/8] Provide a build time default-pager setting
From: Junio C Hamano @ 2009-10-30 22:59 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Junio C Hamano, Ben Walton, Johannes Sixt, David Roundy, GIT List
In-Reply-To: <20091030103925.GI1610@progeny.tock>
I'll queue these for now probably on 'pu', but with the comments we saw on
the list expect them to be followed up with replacement patches.
Thanks.
^ permalink raw reply
* Re: [RFC PATCH v4 26/26] test smart http fetch and push
From: Shawn O. Pearce @ 2009-10-30 22:59 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vljish69m.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> wrote:
> "Shawn O. Pearce" <spearce@spearce.org> writes:
> > Junio C Hamano <gitster@pobox.com> wrote:
> >> "Shawn O. Pearce" <spearce@spearce.org> writes:
> >>
> >> > + sed -e "
> >> > + s/
> >> > //
> >>
> >> This chomped line is so unlike you---what happened?
>
> What I meant was the patch corruption. I couldn't tell what you were
> trying to filter with the first substitution you are giving to sed.
Oh, that. Its a literal CR on the search side, nothing on the
replace side.
--
Shawn.
^ permalink raw reply
* Re: [PATCH 3/8] Teach git var about GIT_EDITOR
From: Junio C Hamano @ 2009-10-30 22:43 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Johannes Sixt, Junio C Hamano, Ben Walton, David Roundy, GIT List
In-Reply-To: <20091030224737.GA16565@progeny.tock>
Jonathan Nieder <jrnieder@gmail.com> writes:
> I am a bit uncomfortable with this error in general. It makes some
> sense to refuse to use $VISUAL and fall back to $EDITOR if TERM=dumb,
> since without this the distinction between VISUAL and EDITOR is not
> very useful.
More importantly, that is what people traditionally expected from VISUAL
and EDITOR and we do that only to follow suit.
There is no such tradition for GIT_EDITOR nor core.editor and switching
behaviour based on the name of editor ("vi"? "vim"?...) does not feel
quite right.
^ permalink raw reply
* Re: [PATCH 3/8] Teach git var about GIT_EDITOR
From: Jonathan Nieder @ 2009-10-30 22:47 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Junio C Hamano, Ben Walton, David Roundy, GIT List
In-Reply-To: <4AEB51C6.7060204@kdbg.org>
Johannes Sixt wrote:
> Jonathan Nieder schrieb:
>> From: Johannes Sixt <j.sixt@viscovery.net>
[...]
> Thanks for cleaning up behind me. I don't mind if you take
> authorship, but if you do keep my name, please make it:
>
> From: Johannes Sixt <j6t@kdbg.org>
Thanks for the catch.
>>+ error(
>> "No editor specified in GIT_EDITOR, core.editor, VISUAL,\n"
>> "or EDITOR. Tried to fall back to vi but terminal is dumb.\n"
>> "Please set one of these variables to an appropriate\n"
>> "editor or run again with options that will not cause an\n"
>> "editor to be invoked (e.g., -m or -F for git commit).");
>>+ return NULL;
>>+ }
>
> I somehow dislike that this huge error message is in git_editor().
Makes sense.
I am a bit uncomfortable with this error in general. It makes some
sense to refuse to use $VISUAL and fall back to $EDITOR if TERM=dumb,
since without this the distinction between VISUAL and EDITOR is not
very useful. But wouldn’t that check be equally useful if GIT_EDITOR
or core.editor is set to vi? Ideally, vi itself would check TERM and
error out, and git would only need to report and handle the exit.
Unfortunately, at least vim is happy to assume a terminal supports
ANSI sequences even if TERM=dumb (e.g., when running from a text
editor like Acme). Unless VISUAL, GIT_EDITOR, and core.editor are
unset, nobody gets the benefit of this check.
Should git error out rather than run $VISUAL when TERM=dumb? How
about $GIT_EDITOR?
The advice about options to avoid invoking an editor is not very
helpful except with 'git commit', so probably only 'git commit' should
print that message.
> The return value, NULL, should be indication enough for the callers
> to handle the situation suitable.
Good idea.
> In particular, launch_editor()
> wants to write this big warning, but 'git var -l' can avoid the
> error message and write only a short notice:
>
> GIT_EDITOR=terminal is dumb, but VISUAL and EDITOR unset
Maybe 'git var -l' should omit GIT_EDITOR in this situation.
Thanks for the comments,
Jonathan
^ permalink raw reply
* Re: [PATCH] Changed timestamp behavior of options -c/-C/--amend
From: Junio C Hamano @ 2009-10-30 22:33 UTC (permalink / raw)
To: Erick Mattos; +Cc: Junio C Hamano, git
In-Reply-To: <55bacdd30910301520h2678d0c2hd8478716d8ce4a17@mail.gmail.com>
Erick Mattos <erick.mattos@gmail.com> writes:
> 2009/10/30 Junio C Hamano <gitster@pobox.com>:
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>>> ...
>>> I agree that the issue the patch addresses is worth improving, and I think
>>> it is sensible to default to reuse the timestamp for -C and not to reuse
>>> for --amend. I am not sure about -c myself, but it probably shouldn't
>>> reuse the timestamp by default.
>>
>> So after realizing that this was about "author" timestamp, I am rescinding
>> this comment about the change of the default for -c and --amend.
>
> Actually I am only changing the default for -c and I see it useful.
> At least with me I normally use -c only to use messages of commits as
> template.
I do that from time to time as well. As I said in a different message, it
may make the default more intutitive if we give new timestamp when the
author is the same as the committer when doing "-c". You are creating
your own commit in that case.
^ permalink raw reply
* Re: [PATCH] Changed timestamp behavior of options -c/-C/--amend
From: Erick Mattos @ 2009-10-30 22:30 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v8wesh61b.fsf@alter.siamese.dyndns.org>
2009/10/30 Junio C Hamano <gitster@pobox.com>:
> Erick Mattos <erick.mattos@gmail.com> writes:
>
>> 2009/10/30 Junio C Hamano <gitster@pobox.com>:
>>
>>>(besides, you write logs as if you are making
>>> an order to the codebase to "do this!").
>>
>> Not a chance! Just trying to help.
>
> Sorry, you misunderstood me. What I meant was:
>
> It is customery for us to write our log messages as if the author of
> the patch is giving an order, iow, in imperative mood. Your "Changed
> blah" violates that style.
>
I got it in the opposite way. :-D
So one should give an "order"!... :-1
^ permalink raw reply
* Re: [PATCH] Changed timestamp behavior of options -c/-C/--amend
From: Junio C Hamano @ 2009-10-30 22:26 UTC (permalink / raw)
To: Erick Mattos; +Cc: git
In-Reply-To: <55bacdd30910301513u6ba6a575w2c65358ff368aeab@mail.gmail.com>
Erick Mattos <erick.mattos@gmail.com> writes:
> 2009/10/30 Junio C Hamano <gitster@pobox.com>:
>
>>(besides, you write logs as if you are making
>> an order to the codebase to "do this!").
>
> Not a chance! Just trying to help.
Sorry, you misunderstood me. What I meant was:
It is customery for us to write our log messages as if the author of
the patch is giving an order, iow, in imperative mood. Your "Changed
blah" violates that style.
^ permalink raw reply
* Re: [RFC PATCH v4 26/26] test smart http fetch and push
From: Junio C Hamano @ 2009-10-30 22:21 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Junio C Hamano, git
In-Reply-To: <20091029143702.GU10505@spearce.org>
"Shawn O. Pearce" <spearce@spearce.org> writes:
> Junio C Hamano <gitster@pobox.com> wrote:
>> "Shawn O. Pearce" <spearce@spearce.org> writes:
>>
>> > +test_expect_success 'clone http repository' '
>> > + GIT_CURL_VERBOSE=1 git clone $HTTPD_URL/git/repo.git clone 2>err &&
>> > + test_cmp file clone/file &&
>> > + egrep "^([<>]|Pragma|Accept|Content-|Transfer-)" err |
>> > + egrep -v "^< (Server|Expires|Date|Content-Length:|Transfer-Encoding: chunked)" |
>> > + sed -e "
>> > + s/
>> > //
>> > + s/^Content-Length: .*$/Content-Length: xxxx/
>> > + " >act &&
>>
>> This chomped line is so unlike you---what happened?
>
> I was getting different Content-Lengths on different runs of the
> test. I don't know why. Here the Content-Length is of the gzip'd
> request, it shouldn't be varying with each run, but it seemed to be.
What I meant was the patch corruption. I couldn't tell what you were
trying to filter with the first substitution you are giving to sed.
^ permalink raw reply
* Re: [PATCH] Changed timestamp behavior of options -c/-C/--amend
From: Erick Mattos @ 2009-10-30 22:20 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7viqdwilx2.fsf@alter.siamese.dyndns.org>
2009/10/30 Junio C Hamano <gitster@pobox.com>:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> ...
>> I agree that the issue the patch addresses is worth improving, and I think
>> it is sensible to default to reuse the timestamp for -C and not to reuse
>> for --amend. I am not sure about -c myself, but it probably shouldn't
>> reuse the timestamp by default.
>
> So after realizing that this was about "author" timestamp, I am rescinding
> this comment about the change of the default for -c and --amend.
Actually I am only changing the default for -c and I see it useful.
At least with me I normally use -c only to use messages of commits as
template.
> But everything else still stands. IOW, I still (1) do think the issue is
> worth addressing (thanks Erick), (2) the log message can be improved, and
> (3) --(old|new)-timestamp should be --[no-]reuse-timestamp.
>
(1) You are very welcome! :-)
(2) As you demand.
(3) I prefer the other way just because of saving some typing and
being more concise in my point of view. But you know you decide it.
^ permalink raw reply
* Re: [PATCH] Changed timestamp behavior of options -c/-C/--amend
From: Erick Mattos @ 2009-10-30 22:13 UTC (permalink / raw)
Cc: git
In-Reply-To: <55bacdd30910301505xe712b74m837dc862a6ee953@mail.gmail.com>
2009/10/30 Junio C Hamano <gitster@pobox.com>:
> Erick Mattos <erick.mattos@gmail.com> writes:
>
> A patch always changes something so the title "Changed ... behavior" does
> not carry enough information
Sorry but I thought It was enough. First submitted patch.
>(besides, you write logs as if you are making
> an order to the codebase to "do this!").
Not a chance! Just trying to help. A way for paying back all the
benefits I enjoy by your software.
>> The code herein changes commit timestamp recording from a source in a
>> more intuitive way.
>>
>> Description:
>
> Remove the above. Instead, start with a description of what the current
> code does, e.g.
>
> Subject: commit -c/-C/--amend: allow 'current' timestamp to be used
>
> When these options are used, the timestamp recorded in the newly
> created commit is always taken from the original commit.
Demand accepted.
>
> Then the rest of your text flows much more nicely...
>
>> When we use one of the options above we are normally trying to do mainly
>> two things: one is using the source as a template and second is to
>> recreate a commit with corrections.
>>
>> In the first case timestamp should by default be taken by the time we
>> are doing the commit, not by the source. On the second case the actual
>> behavior is acceptable.
>
> ... and the reader does not have to wonder what "the actual behavior" is;
> instead you can say "the current behavior" here.
>
>> ...
>> Those options are also useful for --amend option which is by default
>> behaving the same.
>
> Also the reader does not have to wonder what "the same" means here.
Done again.
> I agree that the issue the patch addresses is worth improving, and I think
> it is sensible to default to reuse the timestamp for -C and not to reuse
> for --amend. I am not sure about -c myself, but it probably shouldn't
> reuse the timestamp by default.
>
> I however think (old|new)-timestamp is suboptimal.
>
> We already have --reuse-message, so why not trigger this with a single
> option --(no-)reuse-timestamp?
>
Don't you think it would be a little big? I had compared the option
name so it would be more or less of reuse-message.
^ permalink raw reply
* Re: [PATCH] Changed timestamp behavior of options -c/-C/--amend
From: Johannes Sixt @ 2009-10-30 22:10 UTC (permalink / raw)
To: Jeff King; +Cc: Erick Mattos, Junio C Hamano, git
In-Reply-To: <20091030202628.GA26513@coredump.intra.peff.net>
Jeff King schrieb:
> But this mutual exclusivity implies to me that the option should
> probably be "--timestamp=old|new".
Even better:
--timestamp=now
--timestamp='2008-09-03 13:09:12 +0200'
I'd have needed this just yesterday...
-- Hannes
^ permalink raw reply
* Re: [PATCH] Update packfile transfer protocol documentation
From: Junio C Hamano @ 2009-10-30 22:07 UTC (permalink / raw)
To: Scott Chacon; +Cc: git list, Shawn O. Pearce
In-Reply-To: <d411cc4a0910291035m45ba0a8egd8a991acfbf6d5a7@mail.gmail.com>
Scott Chacon <schacon@gmail.com> writes:
> The technical documentation for the packfile protocol is both sparse and
> incorrect. This documents the fetch-pack/upload-pack and send-pack/
> receive-pack protocols much more fully.
Thanks for starting this. There were a few little things I noticed
(e.g. SHA when it should say SHA-1 or "object name") but we can fix them
up in-tree.
^ permalink raw reply
* Re: [PATCH] Changed timestamp behavior of options -c/-C/--amend
From: Junio C Hamano @ 2009-10-30 21:58 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Erick Mattos, git
In-Reply-To: <7vljisk1m7.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> ...
> I agree that the issue the patch addresses is worth improving, and I think
> it is sensible to default to reuse the timestamp for -C and not to reuse
> for --amend. I am not sure about -c myself, but it probably shouldn't
> reuse the timestamp by default.
So after realizing that this was about "author" timestamp, I am rescinding
this comment about the change of the default for -c and --amend.
But everything else still stands. IOW, I still (1) do think the issue is
worth addressing (thanks Erick), (2) the log message can be improved, and
(3) --(old|new)-timestamp should be --[no-]reuse-timestamp.
^ permalink raw reply
* Re: [PATCH] Changed timestamp behavior of options -c/-C/--amend
From: Johannes Sixt @ 2009-10-30 21:56 UTC (permalink / raw)
To: Erick Mattos; +Cc: Junio C Hamano, git
In-Reply-To: <1256931394-9338-1-git-send-email-erick.mattos@gmail.com>
Erick Mattos schrieb:
> @@ -61,13 +62,16 @@ OPTIONS
> -C <commit>::
> --reuse-message=<commit>::
> Take an existing commit object, and reuse the log message
> - and the authorship information (including the timestamp)
> - when creating the commit.
> + and the authorship information when creating the commit.
> + By default, timestamp is taken from specified commit unless
> + option --new-timestamp is included.
>
> -c <commit>::
> --reedit-message=<commit>::
> Like '-C', but with '-c' the editor is invoked, so that
> the user can further edit the commit message.
> + By default, timestamp is recalculated and not taken from
> + specified commit unless option --old-timestamp is included.
>
> -F <file>::
> --file=<file>::
> @@ -134,6 +138,8 @@ OPTIONS
> current tip -- if it was a merge, it will have the parents of
> the current tip as parents -- so the current top commit is
> discarded.
> + By default, timestamp is taken from latest commit unless option
> + --new-timestamp is included.
I don't like this a lot. Currently, we consistently (and predictably!)
reuse the timestamp when the author information is reused. Therefore, I
think it should be sufficient to introduce only --new-timestamp.
-- Hannes
^ permalink raw reply
* Re: [PATCH] More precise description of 'git describe --abbrev'
From: Junio C Hamano @ 2009-10-30 21:53 UTC (permalink / raw)
To: Andreas Schwab; +Cc: Gisle Aas, git, gitster
In-Reply-To: <m2my38fxeo.fsf@igel.home>
Andreas Schwab <schwab@linux-m68k.org> writes:
> Gisle Aas <gisle@aas.no> writes:
>
>> diff --git a/Documentation/git-describe.txt b/Documentation/git-describe.txt
>> index b231dbb..743eb95 100644
>> --- a/Documentation/git-describe.txt
>> +++ b/Documentation/git-describe.txt
>> @@ -44,7 +44,9 @@ OPTIONS
>>
>> --abbrev=<n>::
>> Instead of using the default 7 hexadecimal digits as the
>> - abbreviated object name, use <n> digits.
>> + abbreviated object name, use <n> digits or as many digits
>> + are needed to form a unique object name. An <n> of 0
>
> "as many digits as needed"?
Thanks; queued with this fix-up.
^ permalink raw reply
* Re: My custom cccmd
From: Junio C Hamano @ 2009-10-30 21:52 UTC (permalink / raw)
To: Felipe Contreras; +Cc: Junio C Hamano, git
In-Reply-To: <94a0d4530910300139l2f20e3aaw2f89e0b809a7246c@mail.gmail.com>
Felipe Contreras <felipe.contreras@gmail.com> writes:
> On Tue, Oct 27, 2009 at 11:53 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Felipe Contreras <felipe.contreras@gmail.com> writes:
>>
>>> I explored this a bit more and I've come to the conclusion that we
>>> actually don't wand to discard the previous commits in the patch
>>> series. Let's think about this example:
>>> 0001 <- John
>>> 0002 <- Me overriding some changes from John
>>>
>>> In this case we want John to appear in the CC list of 0002, because we
>>> are changing his code.
>> ...
>> In such a case, I would imagine that the reviewers would want to see a
>> cleaned up series that does not have his patch that is irrelevant for
>> understanding the final result. John might want to know about it, if only
>> to raise objection to the way how you arranged your series. For that
>> reason, I think it makes sense to Cc him.
>>
>> But it is a rather a convoluted logic, if you ask me. You find John and
>> Cc him, primarily so that he can point out that you should be redoing the
>> series not to have his patch as an intermediate state in the series to
>> begin with, because his commit does not contribute to the end result.
>>
>> It might make more sense if your tool told you about such a case directly,
>> rather than helping you find John so that he can tell you ;-).
>
> But that's not the purpose of the cccmd tool.
>
> I agree that such "patch series simplificator" tool would be very
> useful, but that's out of scope for this. Isn't it?
Exactly.
So you agree that you _do_ want to "discard the previous commits in the
patch series", because not doing so would mean the result would be a
half-cooked "patch series simplificator" that tries to do something that
is outside the scope of cccmd, right?
The "discarding the previous commits" happens to match what I suggested
earlier that lead to your "explored this a bit more":
On Thu, Oct 15, 2009 at 11:37 PM, Felipe Contreras <felipe.contreras@gmail.com> wrote:
> On Thu, Oct 15, 2009 at 11:09 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> #2. If you have two patch series that updates one file twice, some
>> changes in your second patch could even be an update to the changes
>> you introduced in your first patch. After you fix issue #1, you
>> would probably want to fix this by excluding the commits you have
>> already sent the blames for.
so I think we are in agreement.
^ permalink raw reply
* Re: [PATCH] Changed timestamp behavior of options -c/-C/--amend
From: Junio C Hamano @ 2009-10-30 21:51 UTC (permalink / raw)
To: Jeff King; +Cc: Erick Mattos, Junio C Hamano, git
In-Reply-To: <20091030202628.GA26513@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Fri, Oct 30, 2009 at 05:36:34PM -0200, Erick Mattos wrote:
>
>> Anyway this update creates new options for choosing the source timestamp
>> or a new one. And set as default for -c option (editing one) to take a
>> new timestamp and for -C option the source timestamp. That is because
>> we are normally using the source as template when we we are editing and
>> as a correction when we are just copying it.
>>
>> Those options are also useful for --amend option which is by default
>> behaving the same.
>
> Thanks, this is something I have been wanting. I have always thought
> that --amend should give a new timestamp, so that while I'm fixing up
> commits via "rebase -i" the commits end up in correct date order.
>
> Your patch seems to always use the old timestamp for -C, the new one for
> -c, and the old one for --amend. I would want it always for --amend.
>
> I talked with Shawn about this at the GitTogether; his counter-argument
> was that many people in maintainer roles will be amending or rebasing
> just to do little things, like marking Signed-off-by, and that the date
> should remain the same.
Yeah, author timestamp shouldn't be molested for that kind of thing,
although we should update commit timestamp.
Yuck, was this about author timestamp? Please then disregard my previous
response about the default. I do not think there is strong reason to
change the default for any of them at all, even though giving people to
update what they committed with --no-reuse-timestamp would be a good
addition.
I also suspect that comparing committer and author name may give us a good
way to tweak the default in a more user friendly way.
^ permalink raw reply
* Re: [PATCH] gitignore: root most patterns at the top-level directory
From: James Pickens @ 2009-10-30 21:52 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20091030182431.GA19901@coredump.intra.peff.net>
On Fri, Oct 30, 2009 at 11:24 AM, Jeff King <peff@peff.net> wrote:
>> If we do so, there is no need to change the current .gitignore entires.
>> You need to spell a concrete filename as a glob pattern that matches only
>> one path if you want the recursive behaviour. E.g. if you have a Makefile
>> per subdirectory, each of which generates and includes Makefile.depend
>> file, you would write "Makefile.depen[d]" in the toplevel .gitignore file.
>
> While clever, that use of '[d]' seems unneccessarily obscure to me. Why
> not just give a wildcard for "any subdirectory of me" and do:
>
> Makefile.depend
> **/Makefile.depend
>
> Since "**" is in common use in other systems, it's pretty clear (to me,
> anyway, but then I am the one suggesting the syntax ;) ) what that
> means.
+1 to that. I've often wished for Git to support the ** wildcard, not only in
.gitignore but also in other places like .gitattributes and sparse checkout (if
that feature ever gets completed anyways). It's on my list of "git features I
would work on if I ever had any free time."
James
^ permalink raw reply
* Re: Bug#553296: gitignore broken completely
From: Junio C Hamano @ 2009-10-30 21:41 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Klaus Ethgen, 553296, git
In-Reply-To: <20091030194333.GA4551@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Fri, Oct 30, 2009 at 12:41:27PM -0700, Junio C Hamano wrote:
>
>> I've never understood the use of "ls-files -i" without -o, so in that
>> sense, I have done 2. myself already long time ago.
>>
>> In other words, I do not really care that much, and the choice would be
>> between "0. do not do anything---the patch in question was a bugfix for
>> longstanding insanity" and your "4. -i without -o didn't make much sense
>> but now it does and here is the new meaning".
>
> OK, I think the patch I sent elsewhere in the thread should be applied,
> then, as it should make you, me, and Klaus happy.
Thanks; will queue on top of b5227d8.
^ permalink raw reply
* Re: [PATCH] Don't create the $GIT_DIR/branches directory on init
From: Junio C Hamano @ 2009-10-30 21:35 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: git, spearce, sasa.zivkov
In-Reply-To: <1256923228-18949-1-git-send-email-robin.rosenberg@dewire.com>
Robin Rosenberg <robin.rosenberg@dewire.com> writes:
> Git itself does not even look at this directory. Any tools that
> actually needs it should create it itself.
>
> Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
> ---
> templates/branches-- | 1 -
> 1 files changed, 0 insertions(+), 1 deletions(-)
> delete mode 100644 templates/branches--
>
> Shawn and other wants to stop JGit from creating this directory on
> init with the motivation that newer Git version doesn't create it
> anymore. This patch would make that assertion true.
Cogito now seems really dead ;-).
Unless somebody complains I am Ok to queue this for 1.6.6.
>
> -- robin
>
> diff --git a/templates/branches-- b/templates/branches--
> deleted file mode 100644
> index fae8870..0000000
> --- a/templates/branches--
> +++ /dev/null
> @@ -1 +0,0 @@
> -: this is just to ensure the directory exists.
> --
> 1.6.5.2.102.g1f8896
^ permalink raw reply
* Re: [PATCH] Changed timestamp behavior of options -c/-C/--amend
From: Junio C Hamano @ 2009-10-30 21:34 UTC (permalink / raw)
To: Erick Mattos; +Cc: git
In-Reply-To: <1256931394-9338-1-git-send-email-erick.mattos@gmail.com>
Erick Mattos <erick.mattos@gmail.com> writes:
A patch always changes something so the title "Changed ... behavior" does
not carry enough information (besides, you write logs as if you are making
an order to the codebase to "do this!").
> The code herein changes commit timestamp recording from a source in a
> more intuitive way.
>
> Description:
Remove the above. Instead, start with a description of what the current
code does, e.g.
Subject: commit -c/-C/--amend: allow 'current' timestamp to be used
When these options are used, the timestamp recorded in the newly
created commit is always taken from the original commit.
Then the rest of your text flows much more nicely...
> When we use one of the options above we are normally trying to do mainly
> two things: one is using the source as a template and second is to
> recreate a commit with corrections.
>
> In the first case timestamp should by default be taken by the time we
> are doing the commit, not by the source. On the second case the actual
> behavior is acceptable.
... and the reader does not have to wonder what "the actual behavior" is;
instead you can say "the current behavior" here.
> ...
> Those options are also useful for --amend option which is by default
> behaving the same.
Also the reader does not have to wonder what "the same" means here.
I agree that the issue the patch addresses is worth improving, and I think
it is sensible to default to reuse the timestamp for -C and not to reuse
for --amend. I am not sure about -c myself, but it probably shouldn't
reuse the timestamp by default.
I however think (old|new)-timestamp is suboptimal.
We already have --reuse-message, so why not trigger this with a single
option --(no-)reuse-timestamp?
^ permalink raw reply
* Re: [PATCH 02/19] Allow fetch to modify refs
From: Sverre Rabbelier @ 2009-10-30 21:24 UTC (permalink / raw)
To: Daniel Barkalow; +Cc: Git List, Johannes Schindelin, Johan Herland
In-Reply-To: <alpine.LNX.2.00.0910301110120.14365@iabervon.org>
Heya,
On Fri, Oct 30, 2009 at 16:16, Daniel Barkalow <barkalow@iabervon.org> wrote:
> Ah, actually, mapped_refs should just be made non-const; unlike "refs",
> it's set from wanted_peer_refs(), which returns a non-const value.
That works too, even better I guess :).
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* Re: [PATCH] Changed timestamp behavior of options -c/-C/--amend
From: Erick Mattos @ 2009-10-30 21:22 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20091030202628.GA26513@coredump.intra.peff.net>
2009/10/30 Jeff King <peff@peff.net>:
> On Fri, Oct 30, 2009 at 05:36:34PM -0200, Erick Mattos wrote:
>
>> Anyway this update creates new options for choosing the source timestamp
>> or a new one. And set as default for -c option (editing one) to take a
>> new timestamp and for -C option the source timestamp. That is because
>> we are normally using the source as template when we we are editing and
>> as a correction when we are just copying it.
>>
>> Those options are also useful for --amend option which is by default
>> behaving the same.
>
> Thanks, this is something I have been wanting. I have always thought
> that --amend should give a new timestamp, so that while I'm fixing up
> commits via "rebase -i" the commits end up in correct date order.
You are welcome!
> Your patch seems to always use the old timestamp for -C, the new one for
> -c, and the old one for --amend. I would want it always for --amend.
About --amend: I didn't want to change the actual behavior and as a
matter of fact it means only adding the --new-timestamp option when
needed anyway.
> I talked with Shawn about this at the GitTogether; his counter-argument
> was that many people in maintainer roles will be amending or rebasing
> just to do little things, like marking Signed-off-by, and that the date
> should remain the same.
>
> So my suspicion is that there are some people who almost always want
> --new-timestamp, and some people who almost always want --old-timestamp,
> depending on their usual workflow. In which case a config option
> probably makes the most sense (but keeping the command-line to override
> the config, of course).
As you know you will find people defending both sides. I am one that
prefers --amend the way it is now. I really think that having an
option to change it is enough to keep peace.
I don't see a need for more changes in config because it would be imho
more complexity for a small need.
Of course you can do some alias in bash for setting mentioned
functionality up! ;-)
>> ---
>> Documentation/git-commit.txt | 10 ++++++++--
>> builtin-commit.c | 15 ++++++++++++---
>> 2 files changed, 20 insertions(+), 5 deletions(-)
>
> Tests?
Yes indeed. It is a change I am using which I thought it would be
useful for other people. The code is quite simple too. Simplicity
always lead to less possibility of bugs. But yes, I had tested it.
>> Take an existing commit object, and reuse the log message
>> - and the authorship information (including the timestamp)
>> - when creating the commit.
>> + and the authorship information when creating the commit.
>> + By default, timestamp is taken from specified commit unless
>> + option --new-timestamp is included.
>
> We should clarify that this is the _author_ timestamp. The committer
> timestamp is always updated. Yes, it is talking about authorship
> information in the previous sentence, but at least I had to read it
> a few times to figure that out. Plus there are some minor English
> tweaks needed, so maybe:
I see your point but I do not think the way it is is confusing. Of
course we will be talking about taste so I let it pass. Anyway who
would think about or want to change the commiter timestamp? Maybe a
fraudster!... :-1
> and the authorship information when creating the commit.
> By default, the author timestamp is taken from the specified commit
> unless the option --new-timestamp is used.
>
>> -c <commit>::
>> --reedit-message=<commit>::
>> Like '-C', but with '-c' the editor is invoked, so that
>> the user can further edit the commit message.
>> + By default, timestamp is recalculated and not taken from
>> + specified commit unless option --old-timestamp is included.
>
> Ditto:
>
> By default, the author timestamp is recalculated and not taken from
> the specified commit unless the option --old-timestamp is used.
>
>> @@ -134,6 +138,8 @@ OPTIONS
>> current tip -- if it was a merge, it will have the parents of
>> the current tip as parents -- so the current top commit is
>> discarded.
>> + By default, timestamp is taken from latest commit unless option
>> + --new-timestamp is included.
>
> And:
>
> By default, the author timestamp is taken from the latest commit
> unless the option --new-timestamp is included.
As previously said I wouldn't change the text but anyway...
>> + if (old_timestamp && new_timestamp)
>> + die("You can not use --old-timesamp and --new-timestamp together.");
>
> Typo: s/samp/stamp/
>
> But this mutual exclusivity implies to me that the option should
> probably be "--timestamp=old|new".
>
> -Peff
>
Now it is a matter of taste again... I prefer the options as I set up
because it is more the way other options are used so it is more
intuitive. We use very little --'option'='something' using git.
Thanks for all your comments. I have appreciated them very much.
The differences in opinion between us about the little details of this
update is not really relevant because we agree in the need of the
customization proposed.
I have really presented it because I thought it would be useful.
So I think the maintainer can accept it as he judges better.
One of the beauties of Free Software is: even if people don't agree on
a subject one can always compile and use the way one wants. So
everybody gets satisfied!
Thank you very much again.
Best regards.
^ permalink raw reply
* Re: [PATCH 3/8] Teach git var about GIT_EDITOR
From: Johannes Sixt @ 2009-10-30 20:51 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Junio C Hamano, Ben Walton, David Roundy, GIT List
In-Reply-To: <20091030102658.GD1610@progeny.tock>
Jonathan Nieder schrieb:
> From: Johannes Sixt <j.sixt@viscovery.net>
>
> Expose the command used by launch_editor() for scripts to use.
> This should allow one to avoid searching for a proper editor
> separately in each command.
>
> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Thanks for cleaning up behind me. I don't mind if you take authorship, but
if you do keep my name, please make it:
From: Johannes Sixt <j6t@kdbg.org>
> -int launch_editor(const char *path, struct strbuf *buffer, const char *const *env)
> +const char *git_editor(void)
> {
> const char *editor, *terminal;
>
> ...
> terminal = getenv("TERM");
> - if (!editor && (!terminal || !strcmp(terminal, "dumb")))
> + if (!editor && (!terminal || !strcmp(terminal, "dumb"))) {
> /* Terminal is dumb but no VISUAL nor EDITOR defined. */
> - return error(
> + error(
> "No editor specified in GIT_EDITOR, core.editor, VISUAL,\n"
> "or EDITOR. Tried to fall back to vi but terminal is dumb.\n"
> "Please set one of these variables to an appropriate\n"
> "editor or run again with options that will not cause an\n"
> "editor to be invoked (e.g., -m or -F for git commit).");
> + return NULL;
> + }
I somehow dislike that this huge error message is in git_editor(). The
return value, NULL, should be indication enough for the callers to handle
the situation suitable. In particular, launch_editor() wants to write this
big warning, but 'git var -l' can avoid the error message and write only a
short notice:
GIT_EDITOR=terminal is dumb, but VISUAL and EDITOR unset
> +static const char *editor(int flag)
> +{
> + const char *pgm = git_editor();
> +
> + if (!pgm && (flag & IDENT_ERROR_ON_NO_NAME))
> + die("cannot find a suitable editor");
> + return pgm;
This should be
return pgm ? pgm : "terminal is dumb, but VISUAL and EDITOR unset";
otherwise, 'git var -l' later trips over printf("%s", NULL).
-- Hannes
^ 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