git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Maarten Bosmans <mkbosmans@gmail.com>
Cc: git@vger.kernel.org,  Teng Long <dyroneteng@gmail.com>,
	 Maarten Bosmans <maarten.bosmans@vortech.nl>
Subject: Re: [PATCH 1/4] notes: print note blob to stdout directly
Date: Tue, 06 Feb 2024 09:52:38 -0800	[thread overview]
Message-ID: <xmqqy1bxiiop.fsf@gitster.g> (raw)
In-Reply-To: <CA+CvcKTtcHCCKucQ0h1dnaDAMNfErJ+a1CXEVi=ZE5dv57Tb3A@mail.gmail.com> (Maarten Bosmans's message of "Tue, 6 Feb 2024 10:55:04 +0100")

Maarten Bosmans <mkbosmans@gmail.com> writes:

>> I am not sure if we want to accept an approach that feels somewhat
>> narrow/short sighted, like this patch.  When "git show" learns an
>> improved way to show blob objects, who will update the code this
>> patch touches to teach it to use the same improved way to show the
>> notes?

Another thing I forgot to mention was that I suspected the use of
"git show" was so that we can deal with notes trees whose leaves are
*not* blob objects.  As our "git notes" machinery, including how the
initial contents are populated with "git notes add" and how existing
notes on the same object are merged, all assume and depend on that
the leaves are blobs, we can say "we'll just dump to stdout with
write_in_full()", but who knows what kind of custom crap random
third-party tools and IDEs try to use notes trees to store.

Even if we limit ourselves to blobs, they may not be suitable to be
dumped to tty (or a pager, for that matter), and I can see how
textconv could be used as a way out ("detect that the notes payload
is an image and spawn display" kind of hack in a repository full of
images and notes used to store thumbnails, perhaps).

> That is also a cool idea. That would probably use the functionality of
> the cat-file batch mode, right?

Not really.  I was hoping that "git show" that can take multiple
objects from its command line would directly be used, or with a new
option that gives a separator between these objects.

Perhaps we want a new option, e.g., "git notes show --text" that
passes the contents of the leaf blob object to write_in_full(),
bypassing all the things "git show" does, while in a rare case when
the leaf we find is not a blob to invoke "git show".  That might be
a safe approach to move forward, if we wanted to do this.

  reply	other threads:[~2024-02-06 17:52 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-05 20:49 [PATCH 0/4] Speed up git-notes show Maarten Bosmans
2024-02-05 20:49 ` [PATCH 1/4] notes: print note blob to stdout directly Maarten Bosmans
2024-02-06  3:44   ` Junio C Hamano
2024-02-06  9:55     ` Maarten Bosmans
2024-02-06 17:52       ` Junio C Hamano [this message]
2024-02-13  8:00         ` Jeff King
2024-02-13 17:35           ` Junio C Hamano
2024-02-15  5:26             ` Jeff King
2024-02-16  6:25               ` Junio C Hamano
2024-02-17  5:16                 ` Jeff King
2024-02-17  5:56                   ` Junio C Hamano
2024-02-17  6:09                     ` Jeff King
2024-02-15  7:46           ` Maarten Bosmans
2024-02-15 15:04             ` Jeff King
2024-02-17 12:45               ` Maarten Bosmans
2024-02-20  1:51                 ` Jeff King
2024-02-15  7:41         ` Maarten Bosmans
2024-02-06 13:55     ` Kristoffer Haugsbakk
2024-02-05 20:49 ` [PATCH 2/4] notes: use exisisting function stream_blob_to_fd Maarten Bosmans
2024-02-05 22:00   ` Eric Sunshine
2024-02-05 20:49 ` [PATCH 3/4] notes: do not clean up right before calling die() Maarten Bosmans
2024-02-05 20:49 ` [PATCH 4/4] notes: use strbuf_attach to take ownership of the object contents Maarten Bosmans
2024-02-06  7:08 ` [PATCH 0/4] Speed up git-notes show Kristoffer Haugsbakk
2024-02-06  8:51   ` Maarten Bosmans
2024-02-18 19:59 ` [PATCH v2 0/5] " Maarten Bosmans
2024-02-18 19:59   ` [PATCH v2 1/5] log: Move show_blob_object() to log.c Maarten Bosmans
2024-02-20  1:22     ` Junio C Hamano
2024-02-20  1:59       ` Jeff King
2024-02-20  3:03         ` Junio C Hamano
2024-02-20 11:40         ` Maarten Bosmans
2024-02-18 19:59   ` [PATCH v2 2/5] notes: avoid launching a child process to show a note blob Maarten Bosmans
2024-02-18 19:59   ` [PATCH v2 3/5] notes: use existing function stream_blob_to_fd Maarten Bosmans
2024-02-18 19:59   ` [PATCH v2 4/5] notes: do not clean up right before calling die() Maarten Bosmans
2024-02-18 19:59   ` [PATCH v2 5/5] notes: use strbuf_attach to take ownership of the object contents Maarten Bosmans
2024-02-20  2:12     ` Jeff King
2024-02-20  7:42       ` Maarten Bosmans

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=xmqqy1bxiiop.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=dyroneteng@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=maarten.bosmans@vortech.nl \
    --cc=mkbosmans@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
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;
as well as URLs for NNTP newsgroup(s).