From: Michael Fallows <michael@fallo.ws>
To: Jonathan Nieder <jrnieder@gmail.com>
Cc: git@vger.kernel.org, gitster@pobox.com
Subject: Re: [PATCH v2] git.c: Remove unnecessary new line
Date: Sun, 10 Mar 2013 00:10:45 +0000 [thread overview]
Message-ID: <513BCF85.7050605@fallo.ws> (raw)
In-Reply-To: <20130310000023.GI3908@elie.Belkin>
On 10/03/13 00:00, Jonathan Nieder wrote:
> Hi,
>
> Michael Fallows wrote:
>
>> --- a/git.c
>> +++ b/git.c
>> @@ -316,8 +316,7 @@ static void handle_internal_command(int argc, const char **argv)
>> { "check-ignore", cmd_check_ignore, RUN_SETUP | NEED_WORK_TREE },
>> { "check-ref-format", cmd_check_ref_format },
>> { "checkout", cmd_checkout, RUN_SETUP | NEED_WORK_TREE },
>> - { "checkout-index", cmd_checkout_index,
>> - RUN_SETUP | NEED_WORK_TREE},
>> + { "checkout-index", cmd_checkout_index, RUN_SETUP | NEED_WORK_TREE },
>
> This wrapped line was introduced a while ago (4465f410, checkout-index
> needs a working tree, 2007-08-04). It was the first line to wrap, but
> it was also the longest line at the time.
>
> Now the longest line is
>
> { "merge-recursive-theirs", cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE },
>
> (94 columns), so you are right that consistency would suggest dropping
> the line wrapping for checkout-index.
>
> But I find it hard to convince myself that alone is worth the churn.
> In what context did you notice this? Is the intent to help scripts to
> parse the commands[] list, or to manipulate it while preserving
> formatting to avoid distractions? Did you notice the broken line
> while reading through and get distracted, or did some syntax
> highlighting tool notice the oddity, or something else?
>
> Hope that helps,
> Jonathan
>
I do agree with you it does seem like a silly small change in the
context of the project! I noticed it when reading through the source
code (I felt working with git is nice but why not see what makes it
tick). I will admit also, have never contributed to git and my C is
nowhere near the standard worthy of any real contribution so this was
also a step for me to see exactly how the world of patch contribution
works too :D.
Thanks,
Michael
prev parent reply other threads:[~2013-03-10 0:11 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-03-09 22:16 [PATCH v2] git.c: Remove unnecessary new line Michael Fallows
2013-03-10 0:00 ` Jonathan Nieder
2013-03-10 0:10 ` Michael Fallows [this message]
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=513BCF85.7050605@fallo.ws \
--to=michael@fallo.ws \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=jrnieder@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.