Thread (14 messages) 14 messages, 4 authors, 2023-10-31
STALE968d
Revisions (2)
  1. rfc current
  2. v2 [diff vs current]

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

From: Jonathan Tan <hidden>
Date: 2023-10-30 20:22:56

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help