git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] git.c: Remove unnecessary new line
@ 2013-03-09 22:16 Michael Fallows
  2013-03-10  0:00 ` Jonathan Nieder
  0 siblings, 1 reply; 3+ messages in thread
From: Michael Fallows @ 2013-03-09 22:16 UTC (permalink / raw)
  To: git; +Cc: gitster

New line on checkout-index is inconsistent with the rest of the commands

Signed-off-by: Michael Fallows <michael@fallo.ws>
---
 git.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/git.c b/git.c
index b10c18b..b4d7bbb 100644
--- 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 },
 		{ "cherry", cmd_cherry, RUN_SETUP },
 		{ "cherry-pick", cmd_cherry_pick, RUN_SETUP | NEED_WORK_TREE },
 		{ "clean", cmd_clean, RUN_SETUP | NEED_WORK_TREE },
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] git.c: Remove unnecessary new line
  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
  0 siblings, 1 reply; 3+ messages in thread
From: Jonathan Nieder @ 2013-03-10  0:00 UTC (permalink / raw)
  To: Michael Fallows; +Cc: git, gitster

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] git.c: Remove unnecessary new line
  2013-03-10  0:00 ` Jonathan Nieder
@ 2013-03-10  0:10   ` Michael Fallows
  0 siblings, 0 replies; 3+ messages in thread
From: Michael Fallows @ 2013-03-10  0:10 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: git, gitster

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2013-03-10  0:11 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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 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).