git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andreas Ericsson <ae@op5.se>
To: Jeff King <peff@peff.net>
Cc: Wincent Colaiuta <win@wincent.com>,
	Pieter de Bie <pdebie@ai.rug.nl>,
	Junio C Hamano <gitster@pobox.com>,
	Git Mailinglist <git@vger.kernel.org>
Subject: Re: [PATCH] Builtin-commit: show on which branch a commit was added
Date: Wed, 01 Oct 2008 10:13:34 +0200	[thread overview]
Message-ID: <48E3312E.4090601@op5.se> (raw)
In-Reply-To: <20081001031400.GA24513@coredump.intra.peff.net>

Jeff King wrote:
> On Tue, Sep 30, 2008 at 11:59:25AM +0200, Andreas Ericsson wrote:
> 
>> I agree. Obvious solution is to do
>>
>> subj_len = term_width - (strlen(cruft) + strlen(branch_name))
> 
> I think the difficulty is that the printing is sometimes done by our
> printf and sometimes by log_tree_commit, and there isn't a convenient
> way to hook into log_tree_commit to postprocess the formatted output.
> 
>> where strlen(cruft) is just 8 less if we drop 'commit ' from the
>> cases. See the patch I just sent though. I sort of like that one.
> 
> I like it much better than what is on next (and I thought your commit
> message summed up the issue nicely), but...
> 

Thanks. Feel free to recycle it :)

>> Another way would be to write
>> <branch>: Created <hash>: "subject line..."
> 
> I think I like this even better.

Me too, but I thought it up after I sent out the first patch. The nicest
part is that the info that's always present will always end up in the
same place, while my patch moves the branch-name around depending on
the length of the subject line.

Let's agree here and now that the subject should be last and that "commit "
should be dropped, at least for the normal cases.

> My only concern is that many programs
> say "program: some error", so you could potentially have a confusing
> branch name. But I personally have never used a branch name that would
> cause such confusion.
> 

A valid concern, certainly. We needn't use colons for the branch-name
though, but could instead use some other delimiter, like this:
[<branch>] Created <hash>: "subject line..."
although I do believe we're close to nitpicking this issue to death
now. It's not *that* important after all.

>> As <hash> will very, very rarely match anything the user would put
>> in his/her commit message themselves. Quoting the subject is probably
>> a nice touch, and it can make sense to put it last as it's the least
>> interesting of the things we print. Ah well. I'll just await commentary
>> on the patch I've already sent before I go ahead and do something like
>> that.
> 
> Here is a patch for that format on top of next (the patch between this
> and what is in master is even more simple, since we are mostly removing
> Pieter's helper function).
> 

I don't quite like the fact that you're removing the "detached" thingie.
I have coworkers that have been bitten by committing on detached head,
so I'd like to have some mention of it. I'll rework it to take that
into account. Otherwise, this looks good. Less code is always a good
thing, imo.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

  reply	other threads:[~2008-10-01  8:15 UTC|newest]

Thread overview: 67+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-09-02 19:31 [RFC] Detached-HEAD reminder on commit? Pieter de Bie
2008-09-02 19:43 ` Robin Rosenberg
2008-09-02 20:24 ` Nicolas Pitre
2008-09-02 20:26 ` Matthieu Moy
2008-09-02 20:35   ` Nicolas Pitre
2008-09-02 20:39 ` Junio C Hamano
2008-09-02 21:05   ` Stephan Beyer
2008-09-02 21:39     ` Johan Herland
2008-09-02 21:44       ` Jeff King
2008-09-02 21:51         ` Jeff King
2008-09-03  7:45         ` Johan Herland
2008-09-03  8:07           ` Junio C Hamano
2008-09-03  9:47             ` Johan Herland
2008-09-03 13:15           ` Jeff King
2008-09-03 13:34             ` Jeff King
2008-09-03 13:46               ` Andreas Ericsson
2008-09-03 16:49               ` Daniel Barkalow
2008-09-03 18:07                 ` Jeff King
2008-09-03 19:36                   ` Junio C Hamano
2008-09-03 19:41                     ` Jeff King
2008-09-03 14:11             ` Wincent Colaiuta
2008-09-03 18:08               ` Jeff King
2008-09-03 15:16           ` Nicolas Pitre
2008-09-02 21:58       ` Junio C Hamano
2008-09-02 22:53       ` Nicolas Pitre
2008-09-04  4:50         ` Avery Pennarun
2008-09-04  5:31           ` Junio C Hamano
2008-09-05 23:43             ` Junio C Hamano
2008-09-02 21:51     ` Pieter de Bie
2008-09-02 22:11       ` Jakub Narebski
2008-09-02 22:50         ` Junio C Hamano
2008-09-02 22:58           ` Nicolas Pitre
2008-09-03 11:27       ` Pieter de Bie
2008-09-05 17:13         ` [PATCH] Builtin-commit: show on which branch a commit was added Pieter de Bie
2008-09-07  5:27           ` Junio C Hamano
2008-09-07  5:59             ` Junio C Hamano
2008-09-07 23:05               ` [PATCH 1/2] pretty.c: add %% format specifier Pieter de Bie
2008-09-07 23:05                 ` [PATCH 2/2] builtin-commit: show on which branch a commit was added Pieter de Bie
2008-09-21 10:42             ` [PATCH] Builtin-commit: " Jeff King
2008-09-29 20:09               ` Pieter de Bie
2008-09-29 22:44                 ` Jeff King
2008-09-30  6:13                   ` Andreas Ericsson
2008-09-30  6:16                     ` Jeff King
2008-09-30  9:45                       ` Andreas Ericsson
2008-09-30  9:52                       ` [PATCH] git commit: Reformat output somewhat Andreas Ericsson
2008-09-30  6:37                     ` [PATCH] Builtin-commit: show on which branch a commit was added Wincent Colaiuta
2008-09-30  7:09                       ` Jeff King
2008-09-30  9:59                         ` Andreas Ericsson
2008-10-01  3:14                           ` Jeff King
2008-10-01  8:13                             ` Andreas Ericsson [this message]
2008-10-01 15:10                               ` Shawn O. Pearce
2008-10-01 15:22                                 ` Andreas Ericsson
2008-10-01 15:25                                 ` Jeff King
2008-10-01 15:36                                   ` Shawn O. Pearce
2008-10-01 15:42                                     ` Jeff King
2008-10-01 15:44                                       ` Shawn O. Pearce
2008-10-01 21:06                                         ` [PATCH] git commit: Repaint the output format bikeshed (again) Andreas Ericsson
2008-10-01 22:06                                           ` Jeff King
2008-10-01 22:31                                             ` Jeff King
2008-10-02  5:40                                               ` Andreas Ericsson
2008-10-02 21:13                                                 ` Jeff King
2008-10-03  0:15                                                   ` Shawn O. Pearce
2008-10-03  4:24                                                     ` Jeff King
2008-10-03 14:09                                                       ` Shawn O. Pearce
2008-10-04  2:13                                                         ` Jeff King
2008-10-02  8:36                                             ` Wincent Colaiuta
2008-10-01 15:18                               ` [PATCH] Builtin-commit: show on which branch a commit was added Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=48E3312E.4090601@op5.se \
    --to=ae@op5.se \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=pdebie@ai.rug.nl \
    --cc=peff@peff.net \
    --cc=win@wincent.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).