git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, Jeff King <peff@peff.net>,
	Taylor Blau <me@ttaylorr.com>,
	Derrick Stolee <derrickstolee@github.com>,
	Johannes Schindelin <Johannes.Schindelin@gmx.de>
Subject: Re: [PATCH v2] format-patch: add --description-file option
Date: Sun, 13 Aug 2023 10:22:41 +0200	[thread overview]
Message-ID: <ZNiS0TrN0mYD6N97@ugly> (raw)
In-Reply-To: <xmqq7cq0oelh.fsf@gitster.g>

On Sat, Aug 12, 2023 at 01:21:46AM -0700, Junio C Hamano wrote:
>Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:
>
>> On Fri, Aug 11, 2023 at 02:38:05PM -0700, Junio C Hamano wrote:
>>>> +	if (strbuf_read_file(buf, desc_file, 2000) < 0)
>>>
>>>You would probably want to do "2000" -> "0" here.
>>>
>> hmm, yeah, i wonder where i got it from, given that there is no
>> precedent. i suppose i simply thought that 2k is a reasonably
>> expectable max size for a description. if you think the default 8k
>> hint is a better idea, then let's go with it.
>
>The suggestion was not about 2000 vs 8kiB,
>
i know. i just mentioned the default for reference. it seems "severely" 
oversized for the task - not that it would actually matter.

>though it seems we stick
>to power of 2 everywhere we are explicit.  Unless we know the exact
>size from .st_size, that is.
>
>It was primarily about this code not having any need to express its
>own preference and go with whatever is the default.
>
arguably, just about every other instance which uses a fixed hint 
doesn't need to, yet some of them do. it's somewhat obvious in the case 
of "tiny" hints, but there are also some cases of 1k. and the 
sequencer's do_commit() uses 2k for the message file, which is "a funny 
coincidence".
so i wonder whether there is some standard to go by.

>> that's a good point. in fact, passing in the description directly
>> would probably fit my use case better ... i just happened to already
>> have the code for creating that temp file anyway (for editing), so i
>> didn't give it a second thought. i can add both options in the same
>> go, given that it's almost no code.
>
>One thing that you may have to be careful about, if you also take
>strings directly from the command line, is what to do with multiple
>of them.  "git commit -m A -m B" that makes A and B separate
>paragraphs with a break between them, I would think, would serve as
>a good model that end-users already understand well.
>
no worries about that, i'd just copy from commit anyway.

however, this points out a potential problem, which makes me have second 
thoughts about my use case ... i would want to pass the entire contents 
in one argument, newlines and quotes included. i know that this is 
inherently ok on unix, but i wonder whether it would work reliably on 
windows (the wrapper script is written in perl)?

regards


  reply	other threads:[~2023-08-13  8:22 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-07 17:09 [PATCH RESEND] format-patch: add --description-file option Oswald Buddenhagen
2023-08-08  6:33 ` Junio C Hamano
2023-08-09 17:15   ` [PATCH v2] " Oswald Buddenhagen
2023-08-11 21:38     ` Junio C Hamano
2023-08-11 22:29       ` Oswald Buddenhagen
2023-08-12  8:21         ` Junio C Hamano
2023-08-13  8:22           ` Oswald Buddenhagen [this message]
2023-08-21 17:07       ` [PATCH v3] " Oswald Buddenhagen
2023-08-21 17:59         ` Junio C Hamano
2023-08-21 22:05         ` Junio C Hamano
2023-08-23 20:01           ` Taylor Blau
2023-09-23 22:14         ` Kristoffer Haugsbakk
2023-09-25 19:01           ` Junio C Hamano
2023-09-25 19:29             ` Kristoffer Haugsbakk

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=ZNiS0TrN0mYD6N97@ugly \
    --to=oswald.buddenhagen@gmx.de \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=derrickstolee@github.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=me@ttaylorr.com \
    --cc=peff@peff.net \
    /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).