git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: Nikolaj Shurkaev <snnicky@gmail.com>,
	Jakub Narebski <jnareb@gmail.com>,
	git@vger.kernel.org
Subject: Re: git log -z doesn't separate commits with NULs
Date: Fri, 24 Feb 2012 12:03:39 -0800	[thread overview]
Message-ID: <7vsji0xalg.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: 20120224095253.GC11846@sigill.intra.peff.net

Jeff King <peff@peff.net> writes:

>> DESCRIPTION
>> -----------
>> @@ -219,6 +220,12 @@ you can use `--suffix=-patch` to get
>> `0001-description-of-my-change-patch`.
>> range are always formatted as creation patches, independently
>> of this flag.
>> 
>> +[\--] <path>...::
>> + Put in patches only those modifications that affect specified files
>> + and folders. It's important to understand that log message of the
>> + commit may become inappropriate because some parts of patch may be
>> + cut off.
>> +
>
> I think that text looks OK. But to my mind, it is not that format-patch
> accepts a path parameter, but rather that it takes arbitrary log-like
> arguments.

The above text is not telling the entire truth, though.

When the command is run with the "--full-diff" option, seleted commits
will be shown in full.  This is useful for example when you want to pick
commits that add a new "frotz" driver, which obviously needs to include
"drivers/frotz/" subdirectory, without missing necessary changes to the
Makefiles in the higher level (e.g. "drivers/Makefile"), e.g.

	git format-patch --full-diff v1.0..v1.1 -- drivers/frotz

In such a case, "some parts may be cut off" does not make the log message
inappropriate.

On the other hand, people often do not use the resulting history of taking
partial patches (i.e. without --full-diff) and feeding them to "am" as-is.
The operation is used merely to give them a starting point for working on
(possibly) an unrelated topic, and the history is further tweaked with
"rebase -i" or even "commit --amend".  It is not "inappropriate" that the
log says more than what the patch does in such a use case.  What the log
says is irrelevant.

> I don't know how well tested every option is, though, so maybe it's not
> a good idea to encourage the use of random options.

I obviously agree and also suspect that the real question is not "how well
tested" but "if it makes sense".

I am reasonably sure that over time the options and features that make
sense in the context of producing something that is useful with "am" have
been already made to work well, but I also am fairly certain that the
coverage of the code to explicitly reject options that do not make sense
in that context would be spotty at best.  For example, did we carefully
design and implement how format-patch should behave when "-z" is given,
or does the code happen to do whatever it happens to do?  If the latter,
did we consider rejecting "-z" when given from the command line and
implement such safety?

  reply	other threads:[~2012-02-24 20:03 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-02-23  9:14 git log -z doesn't separate commits with NULs Nikolaj Shurkaev
2012-02-23 10:02 ` Luke Diamand
2012-02-23 10:27   ` Jeff King
2012-02-23 10:17 ` Johannes Sixt
2012-02-23 12:11   ` Nikolaj Shurkaev
2012-02-23 10:24 ` Jeff King
2012-02-23 12:17   ` Nikolaj Shurkaev
2012-02-23 13:15     ` Jakub Narebski
2012-02-23 13:48       ` Nikolaj Shurkaev
2012-02-23 19:34         ` Jeff King
2012-02-23 20:07           ` Junio C Hamano
2012-02-24  9:21             ` Nikolaj Shurkaev
2012-02-24  9:52               ` Jeff King
2012-02-24 20:03                 ` Junio C Hamano [this message]
2012-02-24 20:46                   ` Jeff King
2012-02-24 21:14                     ` Junio C Hamano
2012-02-24 21:16                       ` Jeff King
2012-03-03 13:41                         ` Nikolaj Shurkaev
2012-02-24 22:11                       ` Jakub Narebski
2012-02-24 22:27                         ` Junio C Hamano
2012-02-23 10:35 ` Andreas Schwab
2012-02-23 12:19   ` Nikolaj Shurkaev

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=7vsji0xalg.fsf@alter.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=jnareb@gmail.com \
    --cc=peff@peff.net \
    --cc=snnicky@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).