All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.