git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 4/5] Full rework of quote_c_style and write_name_quoted.

From
Andreas Ericsson <ae@op5.se>
Date
Sep 19, 2007, 08:08 UTC
Message-ID
<46F0D8E2.5090706@op5.se>
In-Reply-To
<20070919080030.GA28205@artemis.corp>
Pierre Habouzit wrote:
Show 34 quoted lines
> On Wed, Sep 19, 2007 at 06:37:31AM +0000, Andreas Ericsson wrote:
>> Pierre Habouzit wrote:
>>> diff --git a/builtin-blame.c b/builtin-blame.c
>>> index e364b6c..16c0ca8 100644
>>> --- a/builtin-blame.c
>>> +++ b/builtin-blame.c
>>> @@ -1430,8 +1430,7 @@ static void get_commit_info(struct commit *commit,
>>> static void write_filename_info(const char *path)
>>> {
>>> 	printf("filename ");
>>> -	write_name_quoted(NULL, 0, path, 1, stdout);
>>> -	putchar('\n');
>>> +	write_name_quoted(path, stdout, '\n');
>>> }
>>>
>> This looks like a candidate for a macro. I'm not sure if gcc optimizes
>> sibling calls in void functions with -O2, and it doesn't inline without
>> -O3.
> 
>   Well, there is little point. write_name_quoted behaviour changes if
> the last argument is \0 or non-\0 (see patch comment and quote.c code),
> so it does not really matter to inline the "putchar" IMHO.
> 
>>> -static void diff_flush_raw(struct diff_filepair *p,
>>> -			   struct diff_options *options)
>>> +static void diff_flush_raw(struct diff_filepair *p, struct diff_options 
>>> *opt)
>> Parameter rename? I'd have thought the patch was big enough as it is ;-)
> 
>   I'm anal when it comes to code: the rule of the least surprise should
> apply, and consistency is fundamental. And it happens that diff_options
> are always called `opt' in diff.c, except in that place (and it allows
> to write the prototype of the function on one line).
> 

Then perhaps a separate patch for this would have been prudent? I'm not against the change per se and I understand the reasoning behind it, but it seems to go against Documentation/SubmittingPatches (submit one change at a time).

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231
Previous: Pierre HabouzitNext: Pierre Habouzit
Message 23 of 29 in “let's refactor quoting ...”
  1. Pierre HabouzitSep 18, 2007
  2. 1/5 strbuf API additions and enhancements.Pierre Habouzit, Sep 18, 2007
  3. Edgar ToernigSep 19, 2007
  4. Pierre HabouzitSep 19, 2007
  5. Junio C HamanoSep 19, 2007
  6. Johannes SixtSep 20, 2007
  7. Kalle Olavi NiemitaloSep 20, 2007
  8. Jeff KingSep 20, 2007
  9. 2/5 sq_quote_argv and add_to_string rework with strbuf's.Pierre Habouzit, Sep 18, 2007
  10. Junio C HamanoSep 19, 2007
  11. Pierre HabouzitSep 19, 2007
  12. 3/5 Rework unquote_c_style to work on a strbuf.Pierre Habouzit, Sep 18, 2007
  13. Pierre HabouzitSep 19, 2007
  14. Junio C HamanoSep 19, 2007
  15. Pierre HabouzitSep 19, 2007
  16. 5/5 Avoid duplicating memory, and use xmemdupz instead of xstrdup.Pierre Habouzit, Sep 18, 2007
  17. 4/5 Full rework of quote_c_style and write_name_quoted.Pierre Habouzit, Sep 18, 2007
  18. Pierre HabouzitSep 19, 2007
  19. Junio C HamanoSep 19, 2007
  20. Pierre HabouzitSep 19, 2007
  21. Andreas EricssonSep 19, 2007
  22. Pierre HabouzitSep 19, 2007
  23. Andreas EricssonSep 19, 2007
  24. Pierre HabouzitSep 19, 2007
  25. David KastrupSep 19, 2007
  26. Junio C HamanoSep 19, 2007
  27. David KastrupSep 19, 2007
  28. Junio C HamanoSep 19, 2007
  29. Pierre HabouzitSep 19, 2007

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.