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

Re: [RFCv2 01/16] stringlist: add from_space_separated_string

From
Stefan Beller <sbeller@google.com>
Date
Jun 2, 2015, 17:54 UTC
Message-ID
<CAGZ79katiDViBzC8sMJaDqaekc4+KSyTnfdYD-uni9K72iAVpg@mail.gmail.com>
In-Reply-To
<CAPig+cTsBU2t=NJEe16N2CMyYrWgbP-NvoKxNQ9QdPiokXj+hw@mail.gmail.com>
On Tue, Jun 2, 2015 at 8:10 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:
Show 33 quoted lines
> On Tue, Jun 2, 2015 at 5:42 AM, Duy Nguyen <pclouds@gmail.com> wrote:
>> On Tue, Jun 2, 2015 at 7:02 AM, Stefan Beller <sbeller@google.com> wrote:
>>> diff --git a/string-list.h b/string-list.h
>>> index d3809a1..88c18e9 100644
>>> --- a/string-list.h
>>> +++ b/string-list.h
>>> @@ -19,6 +19,7 @@ struct string_list {
>>>  #define STRING_LIST_INIT_DUP   { NULL, 0, 0, 1, NULL }
>>>
>>>  void string_list_init(struct string_list *list, int strdup_strings);
>>> +void from_space_separated_string(struct string_list *list, char *line);
>>
>> The name feels out of place. All functions in here have "string_list"
>> somewhere in their names. The implementation looks very close to
>> string_list_split() but that name's already taken.. Maybe
>> string_list_split_by_space()?
>
> Indeed. If you really want to go the specialized route, splitting only
> on whitespace, then Duy's suggestion makes sense. Alternately,
> string_list_split_ws() might be easily understood while still
> remaining somewhat terse.
>
> However, why make this so specialized? A more generalized function
> could be more widely useful. For instance, you could introduce a
> function very similar to string_list_split() to which you supply
> multiple delimiter characters (as a 'const char *') rather than the
> single delimiter character accepted by string_list_split(). The
> function could be named string_list_split_any() or
> string_list_tokenize().
>
> Also, it's ugly and inconvenient to require the incoming string be
> non-const, and feels as if you're letting the interface be dictated by
> an implementation detail (underlying use of strtok_r).
I see. I think I can even use string_list_split here, and drop this patch.
Thanks for pointing out the flaws!
Previous: Eric SunshineNext: Stefan Beller
Message 5 of 44 in “[RFCv2 00/16] Protocol version 2”
  1. Stefan BellerJun 2, 2015
  2. 01/16 stringlist: add from_space_separated_stringStefan Beller, Jun 2, 2015
  3. Duy NguyenJun 2, 2015
  4. Eric SunshineJun 2, 2015
  5. Stefan BellerJun 2, 2015
  6. 02/16 upload-pack: make client capability parsing code a separate functionStefan Beller, Jun 2, 2015
  7. 03/16 connect: rewrite feature parsing to work on string_listStefan Beller, Jun 2, 2015
  8. Junio C HamanoJun 2, 2015
  9. 04/16 upload-pack-2: Implement the version 2 of upload-packStefan Beller, Jun 2, 2015
  10. Junio C HamanoJun 2, 2015
  11. Stefan BellerJun 2, 2015
  12. 05/16 remote.h: Change get_remote_heads return to voidStefan Beller, Jun 2, 2015
  13. Junio C HamanoJun 2, 2015
  14. Stefan BellerJun 2, 2015
  15. Junio C HamanoJun 2, 2015
  16. 06/16 remote.h: add new struct for optionsStefan Beller, Jun 2, 2015
  17. Junio C HamanoJun 2, 2015
  18. Stefan BellerJun 2, 2015
  19. Junio C HamanoJun 2, 2015
  20. 07/16 transport: add infrastructure to support a protocol version numberStefan Beller, Jun 2, 2015
  21. 08/16 transport: select transport version via command line or configStefan Beller, Jun 2, 2015
  22. 09/16 remote.h: add get_remote_capabilities, request_capabilitiesStefan Beller, Jun 2, 2015
  23. Junio C HamanoJun 2, 2015
  24. 10/16 transport: connect_setup appends protocol version numberStefan Beller, Jun 2, 2015
  25. Duy NguyenJun 2, 2015
  26. Stefan BellerJun 2, 2015
  27. Junio C HamanoJun 2, 2015
  28. Stefan BellerJun 2, 2015
  29. Junio C HamanoJun 2, 2015
  30. 11/16 remote: have preselect_capabilitiesStefan Beller, Jun 2, 2015
  31. Junio C HamanoJun 2, 2015
  32. 12/16 transport: get_refs_via_connect exchanges capabilities before refs.Stefan Beller, Jun 2, 2015
  33. Junio C HamanoJun 2, 2015
  34. Stefan BellerJun 2, 2015
  35. 13/16 fetch-pack: use the configured transport protocolStefan Beller, Jun 2, 2015
  36. Duy NguyenJun 2, 2015
  37. Duy NguyenJun 2, 2015
  38. Ilari LiusvaaraJun 2, 2015
  39. 14/16 t5544: add a test case for the new protocolStefan Beller, Jun 2, 2015
  40. Eric SunshineJun 3, 2015
  41. 15/16 Documentation/technical/pack-protocol: Mention http as possible protocolStefan Beller, Jun 2, 2015
  42. Junio C HamanoJun 2, 2015
  43. Junio C HamanoJun 2, 2015
  44. 16/16 Document protocol version 2Stefan Beller, Jun 2, 2015

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.