public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Jonathan Tan <jonathantanmy@google•com>
To: git@vger•kernel.org
Cc: Jonathan Tan <jonathantanmy@google•com>,
	Junio C Hamano <gitster@pobox•com>,
	 Phillip Wood <phillip.wood123@gmail•com>,
	Dragan Simic <dsimic@manjaro•org>
Subject: [RFC PATCH 0/3] Avoid passing global comment_line_char repeatedly
Date: Mon, 30 Oct 2023 13:22:45 -0700	[thread overview]
Message-ID: <cover.1698696798.git.jonathantanmy@google.com> (raw)
In-Reply-To: <db6702ba-11a7-44c1-af2a-95b080aaeb77@gmail.com>

> 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


  reply	other threads:[~2023-10-30 20:22 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-30  5:10 [PATCH 0/2] Avoid passing global comment_line_char repeatedly Junio C Hamano
2023-10-30  5:10 ` [PATCH 1/2] strbuf_commented_addf(): drop the comment_line_char parameter Junio C Hamano
2023-10-30  5:10 ` [PATCH 2/2] strbuf_add_commented_lines(): " Junio C Hamano
2023-10-30  5:36 ` [PATCH 0/2] Avoid passing global comment_line_char repeatedly Dragan Simic
2023-10-30  9:59 ` Phillip Wood
2023-10-30 20:22   ` Jonathan Tan [this message]
2023-10-30 20:22     ` [RFC PATCH 1/3] strbuf: make add_lines() public Jonathan Tan
2023-10-30 23:53       ` Junio C Hamano
2023-10-31  6:01         ` Junio C Hamano
2023-10-30 20:22     ` [RFC PATCH 2/3] strbuf_commented_addf(): drop the comment_line_char parameter Jonathan Tan
2023-10-31  5:19       ` Junio C Hamano
2023-10-31 22:24         ` Jonathan Tan
2023-10-31 23:54           ` Junio C Hamano
2023-10-30 20:22     ` [RFC PATCH 3/3] strbuf_add_commented_lines(): " Jonathan Tan
2023-10-31 22:28   ` [PATCH v2 0/4] Avoid passing global comment_line_char repeatedly Jonathan Tan
2023-10-31 22:28     ` [PATCH v2 1/4] strbuf_commented_addf(): drop the comment_line_char parameter Jonathan Tan
2023-10-31 22:28     ` [PATCH v2 2/4] strbuf_add_commented_lines(): " Jonathan Tan
2023-10-31 22:28     ` [PATCH v2 3/4] strbuf: make add_lines() public Jonathan Tan
2023-11-01  4:14       ` Junio C Hamano
2023-10-31 22:28     ` [PATCH v2 4/4] strbuf: move env-using functions to environment.c Jonathan Tan
2023-11-01  4:37       ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cover.1698696798.git.jonathantanmy@google.com \
    --to=jonathantanmy@google$(echo .)com \
    --cc=dsimic@manjaro$(echo .)org \
    --cc=git@vger$(echo .)kernel.org \
    --cc=gitster@pobox$(echo .)com \
    --cc=phillip.wood123@gmail$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox