git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Stelian Pop <stelian@popies.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: Mark Drago <markdrago@gmail.com>, git@vger.kernel.org
Subject: Re: [PATCH] hg-to-git: do not include the branch name as the first line of commit msg
Date: Thu, 06 Dec 2007 14:50:43 +0100	[thread overview]
Message-ID: <1196949043.5388.18.camel@galileo> (raw)
In-Reply-To: <7vsl2jcl32.fsf@gitster.siamese.dyndns.org>


Le lundi 03 décembre 2007 à 17:20 -0800, Junio C Hamano a écrit :

> >      (fdcomment, filecomment) = tempfile.mkstemp()
> > -    csetcomment = os.popen('hg log -r %d -v | grep -v ^changeset: | grep -v ^parent: | grep -v ^user: | grep -v ^date | grep -v ^files: | grep -v ^description: | grep -v ^tag:' % cset).read().strip()
> > +    csetcomment = os.popen('hg log -r %d -v | grep -v ^changeset: | grep -v ^parent: | grep -v ^user: | grep -v ^date | grep -v ^files: | grep -v ^description: | grep -v ^tag: | grep -v ^branch:' % cset).read().strip()
> >      os.write(fdcomment, csetcomment)
> >      os.close(fdcomment)
> 
> Isn't this one of the ugliest lines in the whole git.git project, I have
> to wonder?

It probably is, and I take full resposibility for the original
version :). Incremental development woes.

> I also wonder missing colon after "date" is a bug from the original
> version, and assuming that it is, 

It is indeed a bug.

> how about doing something less error
> prone like this?
> 
>         def included(line):
>             keywords = ('changeset', 'parent', 'user', 'date', 'files',
>                         'description', 'tag', 'branch')
>             for kw in keywords:
>                 if line.startswith(kw + ':'):
>                     return 0
>             return 1
> 
>         hglog = os.popen('hg log -r %d -v' % cset).read();
>         csetcomment = '\n'.join(filter(included, hglog.split('\n'))).strip()

Seems great to me.

> If you are excluding _all_ of the <word>: header lines, the "included"
> function may have to become cleverer but much simpler by doing something
> like:
> 
> 	import re
> 	header_re = re.compile(r'^\w+:')
> 	def included(line):
>         	return not header_re.match(line)

I'm afraid something like this will be much more prone to false
positives. 

Maybe an even better alternative, given the way mercurial outputs the
changeset information, is to search for the '^description:' tag and take
all the text that follows.

-- 
Stelian Pop <stelian@popies.net>

  reply	other threads:[~2007-12-06 14:09 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-12-01 17:56 [PATCH] hg-to-git: do not include the branch name as the first line of commit msg Mark Drago
2007-12-04  1:20 ` Junio C Hamano
2007-12-06 13:50   ` Stelian Pop [this message]
2007-12-06 14:36     ` Baz

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=1196949043.5388.18.camel@galileo \
    --to=stelian@popies.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=markdrago@gmail.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).