[RFC PATCH 0/3] Avoid passing global comment_line_char repeatedly

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> While I agree with your reasoning here, I think that parameter was 
> recently added as part of the libification effort - I can't remember 
> exactly why and am too lazy to look it up so I've cc'd Calvin and 
> Johathan instead.

Thanks Phillip for noticing this. Putting on my libification hat,
this was probably because we wanted to remove strbuf's dependency on
environment, so that we wouldn't need to include it in git-std-lib. If
we were to merge these patches, libification would probably still be
doable if we stubbed the global comment_line_char.

Removing my libification hat, I think it's better to solve this issue
by moving the functions into environment.{c,h} instead, following
the example of functions like strbuf_worktree_ref() in worktree.h
and strbuf_utf8_align() in utf8.h that, when operating on both strbuf
and a specific domain, are placed in the domain's header file, not in
strbuf.h. This avoids a situation in which strbuf.h contains everything
string-related.

The main issue with this is that by not centralizing all strbuf-related
functionality, some strbuf-related helper functions that could have been
private now need to be made public, but I think that a similar issue
would be faced if we don't centralize, say, all environment-related
functionality (some environment-related helper functions would have to
be made public, although I didn't encounter this problem with this patch
set).

I've attached some patches to illustrate what I've described above.

Jonathan Tan (1):
  strbuf: make add_lines() public

Junio C Hamano (2):
  strbuf_commented_addf(): drop the comment_line_char parameter
  strbuf_add_commented_lines(): drop the comment_line_char parameter

 add-patch.c          |  8 ++---
 branch.c             |  3 +-
 builtin/branch.c     |  2 +-
 builtin/merge.c      |  8 ++---
 builtin/notes.c      |  9 +++---
 builtin/stripspace.c |  2 +-
 builtin/tag.c        |  4 +--
 commit.c             |  2 +-
 environment.c        | 31 ++++++++++++++++++++
 environment.h        | 14 +++++++++
 fmt-merge-msg.c      |  9 ++----
 rebase-interactive.c |  8 ++---
 sequencer.c          | 14 ++++-----
 strbuf.c             | 69 ++++++++++----------------------------------
 strbuf.h             | 19 ++----------
 wt-status.c          |  6 ++--
 16 files changed, 98 insertions(+), 110 deletions(-)

-- 
2.42.0.820.g83a721a137-goog





[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux