Git development
 help / color / mirror / Atom feed
* Re: [PATCH v2] builtin/branch.c: adjust error messages to coding guidelines
From: Rubén Justo @ 2023-10-21 10:27 UTC (permalink / raw)
  To: Isoken June Ibizugbe, git; +Cc: christian.couder, gitster
In-Reply-To: <331e1ab3-2e8c-497d-a05d-ef197d664188@gmail.com>

On 19-oct-2023 21:20:24, Rubén Justo wrote:

> So, aside from the confusing message, this iteration looks good to me.

Reviewing again the messages in builtin/branch.c, if you finally
re-roll, maybe you want to add: 

diff --git a/builtin/branch.c b/builtin/branch.c
index e7ee9bd0f1..31da889e95 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -777,7 +777,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
        if (!strcmp(head, "HEAD"))
                filter.detached = 1;
        else if (!skip_prefix(head, "refs/heads/", &head))
-               die(_("HEAD not found below refs/heads!"));
+               die(_("HEAD not found below refs/heads"));
 
But if you don't include this nit, that's fine with me.  I still think
the changes you've already made are correct.

Thank you.

^ permalink raw reply related

* Re: [PATCH v2] builtin/branch.c: adjust error messages to coding guidelines
From: Rubén Justo @ 2023-10-21 10:31 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Isoken June Ibizugbe, git, christian.couder
In-Reply-To: <xmqqwmvhqjyx.fsf@gitster.g>

On 20-oct-2023 10:31:18, Junio C Hamano wrote:
> Rubén Justo <rjusto@gmail.com> writes:
> 
> > On 19-oct-2023 09:40:51, Isoken June Ibizugbe wrote:
> >
> >> As per the CodingGuidelines document, it is recommended that a single-line
> >> message provided to error messages such as die(), error() and warning(),
> >
> > This is confusing; some multi-line messages are fixed in this series.

What I expected to see is a re-roll with no mention about
single/multi-line.

> >> should start with a lowercase letter and should not end with a period.
> >> Also this patch fixes the tests broken by the changes.
> 
> "Also this patch fixes the tests broken by the changes" -> "Adjust
> tests to match updated messages".

That is a more palatable description of why the tests are being touched
in this series.  Thanks.

^ permalink raw reply

* Re: [PATCH] typo: fix the typo 'neeed' into 'needed' in the comment under merge-ort.c
From: Junio C Hamano @ 2023-10-21 17:26 UTC (permalink / raw)
  To: 王常新; +Cc: git
In-Reply-To: <DE904895-230C-436E-B7DE-499E6E503DB9@gmail.com>

王常新 <wchangxin824@gmail.com> writes:

> Sorry about that, I am not quite familiar with the process. I mean
> I can receive emails at both @qq.com and @gmail.com, but
> <foril@foril.space> in the signed-off-by trailer in the commit
> message doesn’t actually exist.

I cannot tell you which between these two to use, as I do not know
your situation.  When a contributor works on Git and send a patch as
an employee of a company, sometimes the employer wants to see their
name prominently shown in the commit, and that is why we see commits
by folks working on Git for GitHub for example with their
@GitHub.com addresses, even though they may have personal addresses
at @gmail.com.  When a contribution is made as a hobbist (which I
was back when I started contributing to this project), people seem
to prefer using their personal address over using their work
address, so that the name and address recorded in the commit will
stay with them even when they move on.

Whatever name and address you choose, if you are using GGG, you'd
need to update your commits locally, perhaps like so:

  $ git commit --amend --author="Name <a@dd.re.ss>"

(and in the editor you have a chance to make sure your sign-off
matches the authorship).

After that I would suppose that you force push the result to update
your pull-request and /submit again (I am not a user of GGG, so I
may have got the details wrong).

Thanks.

^ permalink raw reply

* Re: [PATCH 06/11] t: convert tests to not access reflog via the filesystem
From: Junio C Hamano @ 2023-10-21 23:13 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: git, Han-Wen Nienhuys
In-Reply-To: <eaac658bbfd8259ed9a3cce6ca3c8486d6682e8f.1697607222.git.ps@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

> diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
> index 5f505e2f353..b1d2c014132 100755
> --- a/t/t1400-update-ref.sh
> +++ b/t/t1400-update-ref.sh
> @@ -90,7 +90,8 @@ test_expect_success "deleting current branch adds message to HEAD's log" '
>  	git symbolic-ref HEAD $m &&
>  	git update-ref -m delete-$m -d $m &&
>  	test_must_fail git show-ref --verify -q $m &&
> -	grep "delete-$m$" .git/logs/HEAD
> +	test-tool ref-store main for-each-reflog-ent HEAD >actual &&
> +	grep "delete-$m$" actual
>  '
>  
>  test_expect_success "deleting by HEAD adds message to HEAD's log" '
> @@ -99,7 +100,8 @@ test_expect_success "deleting by HEAD adds message to HEAD's log" '
>  	git symbolic-ref HEAD $m &&
>  	git update-ref -m delete-by-head -d HEAD &&
>  	test_must_fail git show-ref --verify -q $m &&
> -	grep "delete-by-head$" .git/logs/HEAD
> +	test-tool ref-store main for-each-reflog-ent HEAD >actual &&
> +	grep "delete-by-head$" actual
>  '

These are quite straight-forward.

>  test_expect_success "verifying $m's log (logged by config)" '
> -	test_when_finished "git update-ref -d $m && rm -rf .git/logs actual expect" &&
> +	test_when_finished "git update-ref -d $m && git reflog expire --expire=all --all && rm -rf actual expect" &&
>  	test-tool ref-store main for-each-reflog-ent $m >actual &&
>  	test_cmp actual expect
>  '

The approach forces us to assume that "git reflog expire" performs
correctly in order to test reflog, but it probably is OK---we'll
notice breakages in "reflog expire" in other tests, hopefully.

^ permalink raw reply

* Re: [PATCH 0/7] log: decorate pseudorefs and other refs
From: Junio C Hamano @ 2023-10-22  0:13 UTC (permalink / raw)
  To: Andy Koppe; +Cc: git
In-Reply-To: <20231019193911.1669705-1-andy.koppe@gmail.com>

Andy Koppe <andy.koppe@gmail.com> writes:

> This patch series adds three slots to the color.decorate.<slot> config
> option:
> - 'symbol' for coloring the punctuation symbols used around the refs in
>   decorations, which currently use the same color as the commit hash.
> - 'ref' for coloring refs other than branches, remote-tracking branches,
>   tags and the stash, which currently are not colored when included in
>   decorations through custom decoration filter options.
> - 'pseudoref' for coloring pseudorefs such as ORIG_HEAD or MERGE_HEAD.
>   Include them in decorations by default.
>
> This series is to replace the 'decorate: add color.decorate.symbols
> config option' patch proposed at:
> https://lore.kernel.org/git/20231003205442.22963-1-andy.koppe@gmail.com

If that is the case, it probably would have been nicer to mark the
series as [PATCH v2].

Also, can you make messages [1/7]..[7/7] replies to [0/7] when you
send them out?  It seems that all 8 of them (including the cover
letter) are replies to the previous round, which looked a bit
unusual.


As to the contents of the series:

 [1/7] nicely lays out the color documentation; I do not think the
       extra verbosity was absolutely needed for existing ones
       (e.g., when a reader sees 'tag', the reader knows the color
       will be applied to tags), but the more exotic ones the series
       will be adding may deserve extra explanation on what they
       are, so I guess it is OK.

 [2/7] is a trivial readability improvement.  It obviously should be
       left outside the scope of this series, but we should notice
       the same pattern in similar color tables (e.g., wt-status.c
       has one, diff.c has another) and perform the same clean-up as
       a #leftoverbits item.

 [3/7] They way _NIL color is used to control the defaulting looked
       a bit unusual, but clever way to use a non-constant color
       defined elsewhere as its default.  A similar trick is used in
       wt-status.c:color() for STATUS_ONBRANCH, so this is nothing
       new.

 [4/7] The name of new member .include added to ref_namespace_info
       will not be understood by anybody unless they are too deeply
       obsessed by decoration mechansim.  As the namespace_info
       covers far wider interest, so a name that *shouts* that it is
       about decoration filter must be used to be understood by
       readers of the code.

       To be quite honest, "decoration filter" is probably a name
       that will not be understood by anybody, but coming up with a
       better name for it is probably outside the scope of this
       series.

 [5/7] I am not sure if "other refs" should be an item in the
       namespace_info array.  If it is truly "catch-all", then
       shouldn't the refs in other namespaces without their own
       decoration (e.g. ones in refs/notes/ and refs/prefetch/) be
       colored in the same way as this new class?  And if so, having
       it as an independent element that sits next to these other
       classes smells like a strange design.

       Another more worrying thing is that existing .ref members are
       designed to never overlap with each other, but this one
       obviously does.  When a caller with a ref (or a pseudoref)
       asks "which namespace does this one belong to", does the
       existing code still do the right thing with this new element?
       Without it, because there was no overlap, an implementation
       can randomly search in the namespace_info table and stop at
       the first hit, but now with the overlapping and widely open
       .ref = "refs/", the implementation of the search must know
       that it is a fallback position (i.e. if it found a match with
       the fallback .ref = "refs/" , unless it looked at all other
       entries that could begin with "refs/" and are more specific,
       it needs to keep going).

 [6/7] This is pretty straight-forward, assuming that the existing
       is_pseudoref_syntax() function does the right thing.  I am
       not sure about that, though.  A refname with '-' is allowed
       to be called a pseudoref???

       Also, not a fault of this patch, but the "_syntax" in its
       name is totally unnecessary, I would think.  At first glance,
       I suspected that the excuse to append _syntax may have been
       to signal the fact that the helper function does not check if
       there actually is such a ref, but examining a few helpers
       defined nearby tells us that such an excuse does not make
       sense:

           int is_per_worktree_ref(const char *) {
		   return starts_with(refname, "refs/worktree/") ||
			  starts_with(refname, "refs/bisect/") ||
			  starts_with(refname, "refs/rewritten/");
	   }
           int is_pseudoref_syntax(const char *);
           int is_current_worktree_ref(const char *ref) {
                   return is_pseudoref_syntax(ref) || is_per_worktree_ref(ref);
           }

       All these three work on the refname and based on what is in
       that refname string, decides what kind of ref it is.  There
       is nothing especially "syntax" about the second one, and we
       should rename it as part of #leftoverbits clean-up effort.

       Another unrelated tangent is that is_per_worktree_ref() shown
       above and the namespace_info array we saw earlier are not
       even aware of each other, which is maintenance nightmare
       waiting to happen.

 [7/7] Allowing pseudorefs to optionally used when decorating might
       be a good idea, but I do not think it is particularly a good
       design decision to enable it by default.  

       Each of them forming a separate "namespace" also looks like a
       poor design, as being able to group multiple things into one
       family and treat them the same way is the primary point of
       "namespace", I would think.  You do not want to say "I want
       to decorate off of ORIG_HEAD and FETCH_HEAD"; instead you
       would want to say "I want to decorate off of any pseudoref".



^ permalink raw reply

* [RFC PATCH 0/2] pretty: add %I formatting for patch-id
From: Michael McClimon @ 2023-10-22  2:27 UTC (permalink / raw)
  To: git; +Cc: Michael McClimon

I'll say up front that this patch doesn't actually work, but I am stuck
on it and interested in continuing, and am hopeful someone can point me
in something closer to the right direction. (Also, the proposed log
messages are not good, and I would not actually submit the patch for
merge consideration with these messages.)

I would like to have a single-command way to get the patch id for a
commit: the thing you'd see in a pipeline like
    git diff-tree --patch-with-raw HEAD | git patch-id

My initial thought was to add a --patch-id flag to git diff-tree, but
then I thought that maybe better would be to add a pretty specifier to
do so, so that (for instance) you could generate patch-ids for
everything in a branch by saying something like
    git log --pretty='%I %H' start..

I have taken a pass at doing so here, but it doesn't _work_, and I'm not
sure why. It seemed like the thing to do was to use commit_patch_id from
patch-ids.c, but I think that either I'm not holding it correctly, or
that it's not fit for purpose. It was added (or rather, made public) in
ded2c097 (patch-ids: make commit_patch_id() a public helper function,
2016-04-26), which was in service of fa2ab86d (format-patch: add
'--base' option to record base tree info, 2016-04-26). It seems like at
the very least, it probably won't work as expected on merge commits.

The thing that is perplexing to me is that it _does_ appear to work on
some commits, for example (where 8b3aa36f is a recent-ish commit from
master, chosen at random):

$ ./git --no-pager show -s --format='%I %H' 8b3aa36f5a7a0c923bc4a28ff19caae78644ae08
ec66952bdef82a1fc6d31c1057195af31c86da48 8b3aa36f5a7a0c923bc4a28ff19caae78644ae08

$ git diff-tree --patch-with-raw 8b3aa36f5a7a0c923bc4a28ff19caae78644ae08 | git patch-id
ec66952bdef82a1fc6d31c1057195af31c86da48 8b3aa36f5a7a0c923bc4a28ff19caae78644ae08

But for other commits, like the one in the test here, it does not. I
have done a bit of investigation, but I would not really call myself a C
programmer and I'm not super familiar with the codebase, so I'm a bit
stuck. I thought maybe at first I wasn't initializing the diff_options
correctly, but I suspect the problem is actually more fundamental than
that.

Anyway: I would be happy to hear any response at all, whether that's 
a) a pointer to some other way of implementing a %I pretty format,
b) an easier-to-implement possible solution (maybe diff-tree --patch-id
would actually be better), or c) saying the thing I want isn't feasible
and I should stick with the pipeline. Thanks!


Michael McClimon (2):
  patch-ids: add const modifier to commit
  pretty: add 'I' placeholder for patch-id

 patch-ids.c                   |  4 ++--
 patch-ids.h                   |  2 +-
 pretty.c                      | 11 +++++++++++
 t/t4205-log-pretty-formats.sh |  7 +++++++
 4 files changed, 21 insertions(+), 3 deletions(-)

-- 
2.42.0.424.gceadf0f3


^ permalink raw reply

* [RFC PATCH 1/2] patch-ids: add const modifier to commit
From: Michael McClimon @ 2023-10-22  2:27 UTC (permalink / raw)
  To: git; +Cc: Michael McClimon
In-Reply-To: <20231022022800.69219-1-michael@mcclimon.org>

These aren't modified by these functions, and I want to use them
somewhere where the commit is in fact a const.

Signed-off-by: Michael McClimon <michael@mcclimon.org>
---
 patch-ids.c | 4 ++--
 patch-ids.h | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/patch-ids.c b/patch-ids.c
index c3e1a0dd..ecfd7ba0 100644
--- a/patch-ids.c
+++ b/patch-ids.c
@@ -6,13 +6,13 @@
 #include "hex.h"
 #include "patch-ids.h"
 
-static int patch_id_defined(struct commit *commit)
+static int patch_id_defined(const struct commit *commit)
 {
 	/* must be 0 or 1 parents */
 	return !commit->parents || !commit->parents->next;
 }
 
-int commit_patch_id(struct commit *commit, struct diff_options *options,
+int commit_patch_id(const struct commit *commit, struct diff_options *options,
 		    struct object_id *oid, int diff_header_only)
 {
 	if (!patch_id_defined(commit))
diff --git a/patch-ids.h b/patch-ids.h
index 490d7393..3f61d88a 100644
--- a/patch-ids.h
+++ b/patch-ids.h
@@ -19,7 +19,7 @@ struct patch_ids {
 	struct diff_options diffopts;
 };
 
-int commit_patch_id(struct commit *commit, struct diff_options *options,
+int commit_patch_id(const struct commit *commit, struct diff_options *options,
 		    struct object_id *oid, int);
 int init_patch_ids(struct repository *, struct patch_ids *);
 int free_patch_ids(struct patch_ids *);
-- 
2.42.0.424.gceadf0f3


^ permalink raw reply related

* [RFC PATCH 2/2] pretty: add 'I' placeholder for patch-id
From: Michael McClimon @ 2023-10-22  2:28 UTC (permalink / raw)
  To: git; +Cc: Michael McClimon
In-Reply-To: <20231022022800.69219-1-michael@mcclimon.org>

This doesn't actually work yet, and the test is probably in the wrong
place, but I think it's sort of close enough to send an RFC patch to ask
some questions.

Signed-off-by: Michael McClimon <michael@mcclimon.org>
---
 pretty.c                      | 11 +++++++++++
 t/t4205-log-pretty-formats.sh |  7 +++++++
 2 files changed, 18 insertions(+)

diff --git a/pretty.c b/pretty.c
index cf964b06..47e2e6e9 100644
--- a/pretty.c
+++ b/pretty.c
@@ -19,6 +19,7 @@
 #include "trailer.h"
 #include "run-command.h"
 #include "object-name.h"
+#include "patch-ids.h"
 
 /*
  * The limit for formatting directives, which enable the caller to append
@@ -1571,6 +1572,16 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
 						 c->pretty_ctx->abbrev);
 		}
 		return 1;
+	case 'I':
+		{
+			struct diff_options diffopt;
+			struct object_id patch_id;
+			repo_diff_setup(the_repository, &diffopt);
+			if (commit_patch_id(commit, &diffopt, &patch_id, 0))
+				die(_("cannot get patch id"));
+			strbuf_addstr(sb, oid_to_hex(&patch_id));
+			return 1;
+		}
 	case 'm':		/* left/right/bottom */
 		strbuf_addstr(sb, get_revision_mark(NULL, commit));
 		return 1;
diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index e3d655e6..1e9fdcfe 100755
--- a/t/t4205-log-pretty-formats.sh
+++ b/t/t4205-log-pretty-formats.sh
@@ -38,6 +38,13 @@ test_expect_success 'set up basic repos' '
 	git config --unset i18n.commitEncoding
 '
 
+# %I placeholder
+test_expect_success '%I placeholder is a patch-id' '
+	git diff-tree --patch-with-raw HEAD | git patch-id >expected &&
+	git show -s --pretty="%I %H" >actual &&
+	test_cmp expected actual
+'
+
 test_expect_success 'alias builtin format' '
 	git log --pretty=oneline >expected &&
 	git config pretty.test-alias oneline &&
-- 
2.42.0.424.gceadf0f3


^ permalink raw reply related

* [PATCH v3] merge-ort.c: fix typo 'neeed' to 'needed'
From: Wangchangxin via GitGitGadget @ 2023-10-22  2:46 UTC (permalink / raw)
  To: git; +Cc: Wangchangxin, 王常新
In-Reply-To: <pull.1592.v2.git.git.1697719216137.gitgitgadget@gmail.com>

From: =?UTF-8?q?=E7=8E=8B=E5=B8=B8=E6=96=B0?= <wchangxin824@gmail.com>

Signed-off-by: 王常新 (Wang Changxin) <wchangxin824@gmail.com>
---
    typo: fix the typo 'neeed' into 'needed' in the comment under merge-o…
    
    the comments on line 2039 under merge-ort.c should be :
    this is needed if we have content merges of content merges rather than
    this is neeed if we have content merges of content merges
    
    fix the typo

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1592%2FforiLLL%2Fcomment_patch-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1592/foriLLL/comment_patch-v3
Pull-Request: https://github.com/git/git/pull/1592

Range-diff vs v2:

 1:  3934ee6b684 ! 1:  9320154b91a merge-ort.c: fix typo 'neeed' to 'needed'
     @@
       ## Metadata ##
     -Author: foril <1571825323@qq.com>
     +Author: 王常新 <wchangxin824@gmail.com>
      
       ## Commit message ##
          merge-ort.c: fix typo 'neeed' to 'needed'
      
     -    Signed-off-by: 王常新 (Wang Changxin) <foril@foril.space>
     +    Signed-off-by: 王常新 (Wang Changxin) <wchangxin824@gmail.com>
      
       ## merge-ort.c ##
      @@ merge-ort.c: static int handle_content_merge(struct merge_options *opt,


 merge-ort.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/merge-ort.c b/merge-ort.c
index 7857ce9fbd1..aee6f7d8173 100644
--- a/merge-ort.c
+++ b/merge-ort.c
@@ -2036,7 +2036,7 @@ static int handle_content_merge(struct merge_options *opt,
 	 * the three blobs to merge on various sides of history.
 	 *
 	 * extra_marker_size is the amount to extend conflict markers in
-	 * ll_merge; this is neeed if we have content merges of content
+	 * ll_merge; this is needed if we have content merges of content
 	 * merges, which happens for example with rename/rename(2to1) and
 	 * rename/add conflicts.
 	 */

base-commit: a9ecda2788e229afc9b611acaa26d0d9d4da53ed
-- 
gitgitgadget

^ permalink raw reply related

* Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands
From: Jacob Stopak @ 2023-10-22  5:52 UTC (permalink / raw)
  To: Dragan Simic; +Cc: git
In-Reply-To: <d3bbe53c3b910f891c80465ea0c3f53f@manjaro.org>

> Frankly, based on my rather broad experience, there are two primary
> categories of the beginners in the world of version control software (VCS),
> be it git or any other product:
> 
> 1) People who are forced to use some VCS at work, and they actually don't
> give a damn about it.
> 2) True enthusiasts who love what they do, and who love expanding their
> knowledge.
>
> For the first category, nothing helps.  

Interesting categorization I didn't think of splitting users that way. I
guess for group 1 that's true, if they are shown a GUI and can run 3
commands that can do what they need, that's all they will ever use.

> For the second category, a nicely
> written tutorial is all they needed to start with, aided later with the man
> pages, Stack Exchange, and perhaps some textbook.

This is the exact way I learned Git and became comfortable and eventually
confident using it. Reflecting on that, I really only started to become
truly confident after understanding the core underlying concepts (maybe
this is obvious / true for anything). And it's always easy once you get it.

However, there is one main benefit of a feature like this, that none of
the other options (man pages, stack exchange, a textbook) can provide:

Since the tool (Git) has access to and knows the exact state of your local
environment, it can provide instant feedback that takes into account that
context. That is immeasurably more helpful than trying to figure out how
to best ask Google your question, and then piecing together your problem
with a similar one some lost soul ran into 10 years ago.

> Please don't get me wrong, I understand your reasoning, but again, it all
> comes down to the two categories described above.  IMHO, the second category
> will likely start turning off the default hints sooner than turning the
> table formatting on.  The first category will choose some GUI anyway.

The default hints are an intersting consideration. I've found them handy
for commands that I use infrequently, and also when I find myself in a
scenario that is not a part of my usual workflow.

And the hint feature does show that Git has some "helper" features to
hold the user's hand at least a little bit.

> No pain, no gain.  That's the ancient mantra, but IMHO it still applies very
> well to many things, and of course not to the first category mentioned
> above.  Nothing applies to that category.

Somehow I do feel some sense of satisfaction at the countless times I've
I've been stuck on some menial issue only to find out it had a stupid
solution I overlooked. It's also just kind of funny in hindsight.

Regardless of a table formatting feature in Git, there is still plenty of
sweet sweet pain to be had with software dev in general.

But in the moment I always do appreciate being able to get past a
roadblock as quickly as possible. I would want a tool I design to be
known for avoiding pain rather than causing it.

> As I already assumed above, the targeted audience will likely start turning
> the default hints off, rather than turning the table formatting on.  Maybe
> I'm wrong there, who knows.

Even if the hints are off (presumably because the user felt confident
enough or annoyed enough to turn them off), sometimes people just run
into a situation they need an extra bit of clarification on.

^ permalink raw reply

* Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands
From: Jacob Stopak @ 2023-10-22  6:04 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Dragan Simic, git
In-Reply-To: <xmqqwmvgoovg.fsf@gitster.g>

On Fri, Oct 20, 2023 at 04:28:19PM -0700, Junio C Hamano wrote:
> 
> You are not alone in feeling the impedance mismatch between the
> intended audience the patch(es) try to help (pointy-clicky GUI
> users) 

I'm sure there's overlap with "pointy-clicky GUI users" but my point
isn't to directly cater to them. I find it intersting to think about
how visual (and ok fine even gui) tools can be used as bridge tools
that can be discarded one the important concepts are solidified, and
maybe resurrected in a moment of stupidity or strife.

It's like yes use the crutch if you need it, but then do it the real
way once you get it.

And altho this is a visual helper feature, it keeps the user within the
terminal, close to the Git cli and may help some subset stay there.

> and the codebase the patch(es) modify (perhaps spartan
> command line interface).

Git does have some comforting features doesn't it? For example the
hints, as well as the nice pretty colored --graph option for log. I'm
sure I'm missing some others? Isn't there a file called pretty.c?! :D

> I did wonder why this is not made as a
> part of sugarcoating the command line interface with some GUI that
> shows what could be added, what has been added, and the stuff in its
> "git status" equivalent.

I'm working on a couple of tools (ok fine basically guis) that include
this feature.

The reason to bring it up here is that a common feedback I got on my
existing tool was "add something like this into git", so I was curious
about what was possible to do from the Git cli.

This would obviously be the best way for the feature to reach the widest
possible audience and get the most use. Any standalone tool I create
would get a teensy fraction of the use that a feature in Git itself would
get, so I figured I'd give it a whirl.

^ permalink raw reply

* Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands
From: Dragan Simic @ 2023-10-22  6:38 UTC (permalink / raw)
  To: Jacob Stopak; +Cc: git
In-Reply-To: <ZTS4jfsU6oS6LW0u.jacob@initialcommit.io>

On 2023-10-22 07:52, Jacob Stopak wrote:
>> Frankly, based on my rather broad experience, there are two primary
>> categories of the beginners in the world of version control software 
>> (VCS),
>> be it git or any other product:
>> 
>> 1) People who are forced to use some VCS at work, and they actually 
>> don't
>> give a damn about it.
>> 2) True enthusiasts who love what they do, and who love expanding 
>> their
>> knowledge.
>> 
>> For the first category, nothing helps.
> 
> Interesting categorization I didn't think of splitting users that way. 
> I
> guess for group 1 that's true, if they are shown a GUI and can run 3
> commands that can do what they need, that's all they will ever use.

Coincidentally, yesterday I was demonstrated for the first time in my 
life the way VS Code works with git, by a member of the first category 
of users.  It's dumbed down to the extreme, hiding pretty much all the 
details and git specifics, which is exactly what the first category 
wants.  Now I understand why the VS Code is so much popular.

The second category, myself included, tends to be genuinely disgusted by 
such an approach that makes everything nearly sterile.  But I do 
understand why many users simply love it that way.  Maybe I digress.

>> For the second category, a nicely
>> written tutorial is all they needed to start with, aided later with 
>> the man
>> pages, Stack Exchange, and perhaps some textbook.
> 
> This is the exact way I learned Git and became comfortable and 
> eventually
> confident using it. Reflecting on that, I really only started to become
> truly confident after understanding the core underlying concepts (maybe
> this is obvious / true for anything). And it's always easy once you get 
> it.

I agree, understanding the internals of some project or product, with 
many or all of its outer layers peeled back, is often the only way to 
really get to know it.

> However, there is one main benefit of a feature like this, that none of
> the other options (man pages, stack exchange, a textbook) can provide:
> 
> Since the tool (Git) has access to and knows the exact state of your 
> local
> environment, it can provide instant feedback that takes into account 
> that
> context. That is immeasurably more helpful than trying to figure out 
> how
> to best ask Google your question, and then piecing together your 
> problem
> with a similar one some lost soul ran into 10 years ago.

True, but I still think that having git put its thoughts into tables is 
actually not helpful.  To be precise, it actually might be helpful, but 
only to the first category of users, who will never reach it.  I mean, 
never say never, but in this case I'm pretty sure it's safe to say it.  
Unfortunately.

>> Please don't get me wrong, I understand your reasoning, but again, it 
>> all
>> comes down to the two categories described above.  IMHO, the second 
>> category
>> will likely start turning off the default hints sooner than turning 
>> the
>> table formatting on.  The first category will choose some GUI anyway.
> 
> The default hints are an intersting consideration. I've found them 
> handy
> for commands that I use infrequently, and also when I find myself in a
> scenario that is not a part of my usual workflow.

The built-in hints are useful without doubt, and in fact I still have at 
least a dozen of them left enabled.

> And the hint feature does show that Git has some "helper" features to
> hold the user's hand at least a little bit.

IMHO, git strikes a very good balance between holding the user's hand 
and leaving them on their own.  For the second category of users, of 
course.

>> No pain, no gain.  That's the ancient mantra, but IMHO it still 
>> applies very
>> well to many things, and of course not to the first category mentioned
>> above.  Nothing applies to that category.
> 
> Somehow I do feel some sense of satisfaction at the countless times 
> I've
> I've been stuck on some menial issue only to find out it had a stupid
> solution I overlooked. It's also just kind of funny in hindsight.
> 
> Regardless of a table formatting feature in Git, there is still plenty 
> of
> sweet sweet pain to be had with software dev in general.
> 
> But in the moment I always do appreciate being able to get past a
> roadblock as quickly as possible. I would want a tool I design to be
> known for avoiding pain rather than causing it.

I agree, software in general shouldn't cause people pain, it should make 
people's lives better.  However, many people expect software these days 
to be some kind of pain killer, which it simply can't be unless dumbed 
down to the extreme.  If you ask any doctor what results from taking 
pain killers for an extended period of time, they'll answer you that 
stronger pain killers will usually become needed.

^ permalink raw reply

* Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands
From: Dragan Simic @ 2023-10-22  6:52 UTC (permalink / raw)
  To: Jacob Stopak; +Cc: Junio C Hamano, git
In-Reply-To: <ZTS7YsxSE8UA+n4G.jacob@initialcommit.io>

On 2023-10-22 08:04, Jacob Stopak wrote:
> On Fri, Oct 20, 2023 at 04:28:19PM -0700, Junio C Hamano wrote:
>> 
>> You are not alone in feeling the impedance mismatch between the
>> intended audience the patch(es) try to help (pointy-clicky GUI
>> users)
> 
> I'm sure there's overlap with "pointy-clicky GUI users" but my point
> isn't to directly cater to them. I find it intersting to think about
> how visual (and ok fine even gui) tools can be used as bridge tools
> that can be discarded one the important concepts are solidified, and
> maybe resurrected in a moment of stupidity or strife.
> 
> It's like yes use the crutch if you need it, but then do it the real
> way once you get it.

Quite frankly, that would be like starting to learn how to drive a car 
by playing GTA 5, or whichever version of GTA it currently popular.  I 
don't think that would work out well for the vast majority of student 
drivers.

> And altho this is a visual helper feature, it keeps the user within the
> terminal, close to the Git cli and may help some subset stay there.

Even if that would work out for some people, it would require the 
formatting into tables to be the default for git, which frankly I'd 
never support.

^ permalink raw reply

* Re: Pulling from a linux box to a Solaris 9 OS
From: Daniel Santos @ 2023-10-22  9:31 UTC (permalink / raw)
  To: git
In-Reply-To: <20231020062738.GA1642714@coredump.intra.peff.net>

Hello,

I set the environment variable GIT_SSH_VARIANT and it pulled with no issues.
Thanks for the help

Regards
Daniel Santos



> On 20 Oct 2023, at 07:27, Jeff King <peff@peff.net> wrote:
> 
> On Fri, Oct 20, 2023 at 12:33:50AM +0000, brian m. carlson wrote:
> 
>> By default, if the SSH binary is the default ("ssh"), Git assumes that
>> it's OpenSSH and sends certain options to enable protocol v2, including
>> -o SendEnv.
>> 
>> If you don't want that, you can set "ssh.variant" to "simple", in which
>> case Git will send only the username and the host, but not -p port, -4,
>> -6, or -o.  If you do need a different port, then you're out of luck,
>> and will either have to install Putty (in which case, the ssh.variant
>> value would need to be "putty") or upgrade OpenSSH.  Otherwise, the
>> simple value should work fine.
> 
> I think your suggestion is the most straight-forward one, but just in
> case the "out of luck" part is a problem, you should also be able to
> side-step the issue with:
> 
>  git -c protocol.version=0 fetch ...
> 
> That would allow other features (assuming this older ssh version
> supports them!) without triggering the SendEnv option.
> 
> -Peff


^ permalink raw reply

* Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands
From: Oswald Buddenhagen @ 2023-10-22 10:30 UTC (permalink / raw)
  To: Dragan Simic; +Cc: Jacob Stopak, git
In-Reply-To: <5fac8607a3c270e06fd610551d7403c7@manjaro.org>

On Sun, Oct 22, 2023 at 08:38:19AM +0200, Dragan Simic wrote:
>True, but I still think that having git put its thoughts into tables is 
>actually not helpful.
>
i'm not convinced that the proposed feature specifically would have 
helped me, either (i found the index a rather obvious concept once i 
knew that it's there), but i'm making a general argument here. so:

>To be precise, it actually might be helpful, but only to the first 
>category of users, who will never reach it.  I mean, never say never, 
>but in this case I'm pretty sure it's safe to say it.  
>
well, and i think that you're wrong about that.
your categorization is simply wrong, because it assumes an incorrect 
static model.

while for the last decade i've been as much of a git expert as one can 
reasonably be without being literally obsessed with it or having written 
much of it, i absolutely *did* start out in your first category (as in, 
it was forced upon me, while i couldn't have cared less about the 
specifics - p4 was working well enough (or so i thought)). and i hated 
this stupid git (it was 2009, and it was much more of a pita for noobs 
than it is now). i certainly could have used more sensible 
visualizations at every step - on the command line, because that's where 
i mostly "live".

the second major error in the thinking is that "expert" and "gui user" 
are mutually exclusive categories. while i do most things on the command 
line, i would never voluntarily use "add -p" - why should i inflict that 
pain upon me, when i can simply use git-gui to do the job in a much more 
visual and freely navigable way? the same goes for "log --graph" vs.  
gitk, and git's "blame" function vs. qt creator's (or git-gui's, but i 
don't use it for that).

regards

^ permalink raw reply

* Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands
From: Dragan Simic @ 2023-10-22 12:55 UTC (permalink / raw)
  To: Oswald Buddenhagen; +Cc: Jacob Stopak, git
In-Reply-To: <ZTT5uI5Hm1+n0Agx@ugly>

On 2023-10-22 12:30, Oswald Buddenhagen wrote:
> On Sun, Oct 22, 2023 at 08:38:19AM +0200, Dragan Simic wrote:
>> True, but I still think that having git put its thoughts into tables
>> is actually not helpful.
> 
> i'm not convinced that the proposed feature specifically would have
> helped me, either (i found the index a rather obvious concept once i
> knew that it's there), but i'm making a general argument here. so:
> 
>> To be precise, it actually might be helpful, but only to the first
>> category of users, who will never reach it.  I mean, never say never,
>> but in this case I'm pretty sure it's safe to say it.
> 
> well, and i think that you're wrong about that.
> your categorization is simply wrong, because it assumes an incorrect
> static model.
> 
> while for the last decade i've been as much of a git expert as one can
> reasonably be without being literally obsessed with it or having
> written much of it, i absolutely *did* start out in your first
> category (as in, it was forced upon me, while i couldn't have cared
> less about the specifics - p4 was working well enough (or so i
> thought)). and i hated this stupid git (it was 2009, and it was much
> more of a pita for noobs than it is now). i certainly could have used
> more sensible visualizations at every step - on the command line,
> because that's where i mostly "live".

Oh, that's awesome and I'm really happy to be wrong with my broad 
classification of VCS users.  However, I still need to be convinced 
further, and I'd assign your example as an exception to the rules, 
especially because you migrated to git from another VCS, which you 
liked, and because you use the command line a lot.

Full disclosure, I used Subversion for many years and I loved it.  I 
knew it very well and it did all I needed for me and the team I worked 
with.  Then git came and I really didn't like it, because it was touted 
to be "the best thing ever".  After using git for a while, I can firmly 
say that git is awesome, but that it also is a total overkill for many 
projects that need a VCS, for which choosing Subversion would be a much 
batter choice.  Why, you'll ask?  Because Subversion is many times 
simpler, and because many projects actually don't need a distributed 
VCS.

> the second major error in the thinking is that "expert" and "gui user"
> are mutually exclusive categories. while i do most things on the
> command line, i would never voluntarily use "add -p" - why should i
> inflict that pain upon me, when i can simply use git-gui to do the job
> in a much more visual and freely navigable way? the same goes for "log
> --graph" vs.  gitk, and git's "blame" function vs. qt creator's (or
> git-gui's, but i don't use it for that).

I also ask myself why would I use git-gui or any other GUI utility?  To 
me, clicking on something that represents a file is often simply wrong.  
Though, I understand that many people prefer GUI utilities and I respect 
that, everyone is free to do anything, but I also expect others to 
respect my own preferences.

^ permalink raw reply

* Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands
From: Jacob Stopak @ 2023-10-22 15:50 UTC (permalink / raw)
  To: Oswald Buddenhagen; +Cc: Dragan Simic, git
In-Reply-To: <ZTT5uI5Hm1+n0Agx@ugly>

On Sun, Oct 22, 2023 at 12:30:16PM +0200, Oswald Buddenhagen wrote:
> On Sun, Oct 22, 2023 at 08:38:19AM +0200, Dragan Simic wrote:
> > True, but I still think that having git put its thoughts into tables is
> > actually not helpful.
> > 
> i'm not convinced that the proposed feature specifically would have helped
> me, either (i found the index a rather obvious concept once i knew that it's
> there), but i'm making a general argument here. so:
> 

Thank you for the input! One point I'd like to add is that although the
current proposal patch series only implements the table option for the
status and add commands, it could be applied to many others as dry runs,
as mentioned in my cover letter, some of which touch on concepts besides
the index. Examples would be git stash and git clean. It can take a while
before all these commands feel natural, which was my hope for having this
optional helper when the user simply could use a bit more clarity.

> > To be precise, it actually might be helpful, but only to the first
> > category of users, who will never reach it.  I mean, never say never,
> > but in this case I'm pretty sure it's safe to say it.
> > 
> well, and i think that you're wrong about that.
> your categorization is simply wrong, because it assumes an incorrect static
> model.
> 
> while for the last decade i've been as much of a git expert as one can
> reasonably be without being literally obsessed with it or having written
> much of it, i absolutely *did* start out in your first category (as in, it
> was forced upon me, while i couldn't have cared less about the specifics -
> p4 was working well enough (or so i thought)). and i hated this stupid git
> (it was 2009, and it was much more of a pita for noobs than it is now). i
> certainly could have used more sensible visualizations at every step - on
> the command line, because that's where i mostly "live".
> 

:D. I feel similar and as mentioned in a reply above, the main benefit
to getting direct feedback on in the cli is that it can provide guidance
based on the exact context the user is in, instead of an external
resource which can in most cases only suggest a more general or
tangential guidance / solution.

^ permalink raw reply

* Outreachy Internship Project Timeline
From: Naomi Ibe @ 2023-10-22 17:16 UTC (permalink / raw)
  To: git

On the Outreachy website under the "Final application section", this
statement appears:
"Please work with your mentor to provide a timeline of the work you
plan to accomplish on the project and what tasks you will finish at
each step. Make sure take into account any time commitments you have
during the Outreachy internship round. If you are still working on
your contributions and need more time, you can leave this blank and
edit your application later."
Is there a link to where I can access the project timeline and
subsequently edit it?

^ permalink raw reply

* [PATCH] doc/git-bisect: clarify `git bisect run` syntax
From: cousteau via GitGitGadget @ 2023-10-22 20:02 UTC (permalink / raw)
  To: git; +Cc: cousteau, Javier Mora

From: Javier Mora <cousteaulecommandant@gmail.com>

The description of the `git bisect run` command syntax at the beginning
of the manpage is `git bisect run <cmd>...`, which isn't quite clear
about what `<cmd>` is or what the `...` mean; one could think that it is
the whole (quoted) command line with all arguments in a single string,
or that it supports multiple commands, or that it doesn't accept
commands with arguments at all.

Change to `git bisect run <cmd> [<arg>...]` to clarify the syntax.

Signed-off-by: Javier Mora <cousteaulecommandant@gmail.com>
---
    doc/git-bisect: clarify git bisect run syntax
    
    I saw someone in IRC wondering about the syntax for git bisect run for a
    command with arguments, and found that its short description at the
    beginning of the manpage is not very clear (although it gets clarified
    later when it is properly described). It describes the syntax as git
    bisect run <cmd>... which is a bit confusing; it should say git bisect
    run <cmd> [<arg>...], otherwise it somehow looks like you have to "enter
    one or more commands", and that each command is a single argument.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1602%2Fcousteaulecommandant%2Fman-git-bisect-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1602/cousteaulecommandant/man-git-bisect-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1602

 Documentation/git-bisect.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
index 7872dba3aef..19bbed49238 100644
--- a/Documentation/git-bisect.txt
+++ b/Documentation/git-bisect.txt
@@ -26,7 +26,7 @@ on the subcommand:
  git bisect (visualize|view)
  git bisect replay <logfile>
  git bisect log
- git bisect run <cmd>...
+ git bisect run <cmd> [<arg>...]
  git bisect help
 
 This command uses a binary search algorithm to find which commit in

base-commit: ceadf0f3cf51550166a387ec8508bb55e7883057
-- 
gitgitgadget

^ permalink raw reply related

* Re: [PATCH] doc/git-bisect: clarify `git bisect run` syntax
From: Eric Sunshine @ 2023-10-22 21:32 UTC (permalink / raw)
  To: cousteau via GitGitGadget; +Cc: git, Javier Mora
In-Reply-To: <pull.1602.git.1698004968582.gitgitgadget@gmail.com>

On Sun, Oct 22, 2023 at 4:03 PM cousteau via GitGitGadget
<gitgitgadget@gmail.com> wrote:
> The description of the `git bisect run` command syntax at the beginning
> of the manpage is `git bisect run <cmd>...`, which isn't quite clear
> about what `<cmd>` is or what the `...` mean; one could think that it is
> the whole (quoted) command line with all arguments in a single string,
> or that it supports multiple commands, or that it doesn't accept
> commands with arguments at all.
>
> Change to `git bisect run <cmd> [<arg>...]` to clarify the syntax.

Okay, makes sense.

> Signed-off-by: Javier Mora <cousteaulecommandant@gmail.com>
> ---
> diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
> @@ -26,7 +26,7 @@ on the subcommand:
> - git bisect run <cmd>...
> + git bisect run <cmd> [<arg>...]

The output of `git bisect -h` suffers the same problem. Perhaps this
patch can fix that, as well?

^ permalink raw reply

* [PATCH v2 1/6] config: restructure color.decorate documentation
From: Andy Koppe @ 2023-10-22 21:44 UTC (permalink / raw)
  To: git; +Cc: gitster, Andy Koppe
In-Reply-To: <20231022214432.56325-1-andy.koppe@gmail.com>

List color.decorate slots in git-config documentation one-by-one in the
same way as color.grep slots, to aid readability and make it easier to
add slots.

Signed-off-by: Andy Koppe <andy.koppe@gmail.com>
---
 Documentation/config/color.txt | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/Documentation/config/color.txt b/Documentation/config/color.txt
index 1795b2d16b..3453703f9b 100644
--- a/Documentation/config/color.txt
+++ b/Documentation/config/color.txt
@@ -74,10 +74,25 @@ color.diff.<slot>::
 	`oldBold`, and `newBold` (see linkgit:git-range-diff[1] for details).
 
 color.decorate.<slot>::
-	Use customized color for 'git log --decorate' output.  `<slot>` is one
-	of `branch`, `remoteBranch`, `tag`, `stash` or `HEAD` for local
-	branches, remote-tracking branches, tags, stash and HEAD, respectively
-	and `grafted` for grafted commits.
+	Use customized color for the output of 'git log --decorate' as well as
+	the `%d`, `%D` and `%(decorate)` placeholders in custom log formats,
+	whereby `<slot>` specifies which decoration elements the color applies
+	to:
++
+--
+`HEAD`;;
+	the current HEAD
+`branch`;;
+	local branches
+`remoteBranch`;;
+	remote-tracking branches
+`tag`;;
+	lightweight and annotated tags
+`stash`;;
+	the stash ref
+`grafted`;;
+	grafted and replaced commits
+--
 
 color.grep::
 	When set to `always`, always highlight matches.  When `false` (or
-- 
2.42.GIT


^ permalink raw reply related

* [PATCH v2 0/6] log: decorate pseudorefs and other refs
From: Andy Koppe @ 2023-10-22 21:44 UTC (permalink / raw)
  To: git; +Cc: gitster, Andy Koppe
In-Reply-To: <20231019193911.1669705-1-andy.koppe@gmail.com>

This patch series implements decoration with pseudorefs and adds three
slots to the color.decorate.<slot> config:
- 'symbol' for coloring the punctuation symbols used around the refs in
  decorations, which currently use the same color as the commit hash.
- 'ref' for coloring refs other than branches, remote-tracking branches,
  tags and the stash, which currently are not colored when included in
  decorations through custom decoration filter options.
- 'pseudoref' for coloring pseudorefs such as ORIG_HEAD or MERGE_HEAD.

CI: https://github.com/ak2/git/actions/runs/6605893645

Andy Koppe (6):
  config: restructure color.decorate documentation
  log: add color.decorate.symbol config variable
  log: add color.decorate.ref config variable
  refs: add pseudorefs array and iteration functions
  refs: exempt pseudorefs from pattern prefixing
  log: add color.decorate.pseudoref config variable

 Documentation/config/color.txt                | 32 +++++++-
 commit.h                                      |  3 +
 log-tree.c                                    | 43 +++++++++--
 refs.c                                        | 59 +++++++++++++--
 refs.h                                        |  5 ++
 ..._--decorate=full_--clear-decorations_--all |  4 +-
 ...f.log_--decorate_--clear-decorations_--all |  4 +-
 t/t4202-log.sh                                | 21 +++---
 t/t4207-log-decoration-colors.sh              | 74 +++++++++++--------
 9 files changed, 185 insertions(+), 60 deletions(-)

-- 
2.42.GIT


^ permalink raw reply

* [PATCH v2 2/6] log: add color.decorate.symbol config variable
From: Andy Koppe @ 2023-10-22 21:44 UTC (permalink / raw)
  To: git; +Cc: gitster, Andy Koppe
In-Reply-To: <20231022214432.56325-1-andy.koppe@gmail.com>

Add new color.decorate.symbol config variable for determining the
color of the prefix, suffix, separator and pointer symbols used in
log --decorate output and related log format placeholders, to allow
them to be colored differently from commit hashes.

For backward compatibility, fall back to the commit hash color that can
be specified with the color.diff.commit variable if the new variable is
not provided.

Add the variable to the color.decorate.<slot> documentation.

Amend t4207-log-decoration-colors.sh to test it. Put ${c_reset} elements
in the expected output at the end of lines for consistency.

Signed-off-by: Andy Koppe <andy.koppe@gmail.com>
---
 Documentation/config/color.txt   |  2 ++
 commit.h                         |  1 +
 log-tree.c                       | 15 ++++++---
 t/t4207-log-decoration-colors.sh | 58 +++++++++++++++++---------------
 4 files changed, 43 insertions(+), 33 deletions(-)

diff --git a/Documentation/config/color.txt b/Documentation/config/color.txt
index 3453703f9b..cc0a881125 100644
--- a/Documentation/config/color.txt
+++ b/Documentation/config/color.txt
@@ -92,6 +92,8 @@ color.decorate.<slot>::
 	the stash ref
 `grafted`;;
 	grafted and replaced commits
+`symbol`;;
+	punctuation symbols surrounding the other elements
 --
 
 color.grep::
diff --git a/commit.h b/commit.h
index 28928833c5..cb13e4d5ba 100644
--- a/commit.h
+++ b/commit.h
@@ -56,6 +56,7 @@ enum decoration_type {
 	DECORATION_REF_STASH,
 	DECORATION_REF_HEAD,
 	DECORATION_GRAFTED,
+	DECORATION_SYMBOL,
 };
 
 void add_name_decoration(enum decoration_type type, const char *name, struct object *obj);
diff --git a/log-tree.c b/log-tree.c
index 504da6b519..5ad168458e 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -41,6 +41,7 @@ static char decoration_colors[][COLOR_MAXLEN] = {
 	GIT_COLOR_BOLD_MAGENTA,	/* REF_STASH */
 	GIT_COLOR_BOLD_CYAN,	/* REF_HEAD */
 	GIT_COLOR_BOLD_BLUE,	/* GRAFTED */
+	GIT_COLOR_NIL,		/* SYMBOL */
 };
 
 static const char *color_decorate_slots[] = {
@@ -50,6 +51,7 @@ static const char *color_decorate_slots[] = {
 	[DECORATION_REF_STASH]	= "stash",
 	[DECORATION_REF_HEAD]	= "HEAD",
 	[DECORATION_GRAFTED]	= "grafted",
+	[DECORATION_SYMBOL]	= "symbol",
 };
 
 static const char *decorate_get_color(int decorate_use_color, enum decoration_type ix)
@@ -312,7 +314,7 @@ void format_decorations(struct strbuf *sb,
 {
 	const struct name_decoration *decoration;
 	const struct name_decoration *current_and_HEAD;
-	const char *color_commit, *color_reset;
+	const char *color_symbol, *color_reset;
 
 	const char *prefix = " (";
 	const char *suffix = ")";
@@ -337,7 +339,10 @@ void format_decorations(struct strbuf *sb,
 			tag = opts->tag;
 	}
 
-	color_commit = diff_get_color(use_color, DIFF_COMMIT);
+	color_symbol = decorate_get_color(use_color, DECORATION_SYMBOL);
+	if (color_is_nil(color_symbol))
+		color_symbol = diff_get_color(use_color, DIFF_COMMIT);
+
 	color_reset = decorate_get_color(use_color, DECORATION_NONE);
 
 	current_and_HEAD = current_pointed_by_HEAD(decoration);
@@ -352,7 +357,7 @@ void format_decorations(struct strbuf *sb,
 				decorate_get_color(use_color, decoration->type);
 
 			if (*prefix) {
-				strbuf_addstr(sb, color_commit);
+				strbuf_addstr(sb, color_symbol);
 				strbuf_addstr(sb, prefix);
 				strbuf_addstr(sb, color_reset);
 			}
@@ -369,7 +374,7 @@ void format_decorations(struct strbuf *sb,
 
 			if (current_and_HEAD &&
 			    decoration->type == DECORATION_REF_HEAD) {
-				strbuf_addstr(sb, color_commit);
+				strbuf_addstr(sb, color_symbol);
 				strbuf_addstr(sb, pointer);
 				strbuf_addstr(sb, color_reset);
 				strbuf_addstr(sb, decorate_get_color(use_color, current_and_HEAD->type));
@@ -382,7 +387,7 @@ void format_decorations(struct strbuf *sb,
 		decoration = decoration->next;
 	}
 	if (*suffix) {
-		strbuf_addstr(sb, color_commit);
+		strbuf_addstr(sb, color_symbol);
 		strbuf_addstr(sb, suffix);
 		strbuf_addstr(sb, color_reset);
 	}
diff --git a/t/t4207-log-decoration-colors.sh b/t/t4207-log-decoration-colors.sh
index 21986a866d..f4173b6114 100755
--- a/t/t4207-log-decoration-colors.sh
+++ b/t/t4207-log-decoration-colors.sh
@@ -18,6 +18,7 @@ test_expect_success setup '
 	git config color.decorate.tag "reverse bold yellow" &&
 	git config color.decorate.stash magenta &&
 	git config color.decorate.grafted black &&
+	git config color.decorate.symbol white &&
 	git config color.decorate.HEAD cyan &&
 
 	c_reset="<RESET>" &&
@@ -29,6 +30,7 @@ test_expect_success setup '
 	c_stash="<MAGENTA>" &&
 	c_HEAD="<CYAN>" &&
 	c_grafted="<BLACK>" &&
+	c_symbol="<WHITE>" &&
 
 	test_commit A &&
 	git clone . other &&
@@ -53,17 +55,17 @@ cmp_filtered_decorations () {
 # to this test since it does not contain any decoration, hence --first-parent
 test_expect_success 'commit decorations colored correctly' '
 	cat >expect <<-EOF &&
-	${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_HEAD}HEAD${c_reset}\
-${c_commit} -> ${c_reset}${c_branch}main${c_reset}${c_commit}, \
-${c_reset}${c_tag}tag: ${c_reset}${c_tag}v1.0${c_reset}${c_commit}, \
-${c_reset}${c_tag}tag: ${c_reset}${c_tag}B${c_reset}${c_commit})${c_reset} B
-${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}\
-${c_tag}tag: ${c_reset}${c_tag}A1${c_reset}${c_commit}, \
-${c_reset}${c_remoteBranch}other/main${c_reset}${c_commit})${c_reset} A1
-	${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}\
-${c_stash}refs/stash${c_reset}${c_commit})${c_reset} On main: Changes to A.t
-	${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}\
-${c_tag}tag: ${c_reset}${c_tag}A${c_reset}${c_commit})${c_reset} A
+	${c_commit}COMMIT_ID${c_reset}${c_symbol} (${c_reset}${c_HEAD}HEAD${c_reset}\
+${c_symbol} -> ${c_reset}${c_branch}main${c_reset}${c_symbol}, ${c_reset}\
+${c_tag}tag: ${c_reset}${c_tag}v1.0${c_reset}${c_symbol}, ${c_reset}\
+${c_tag}tag: ${c_reset}${c_tag}B${c_reset}${c_symbol})${c_reset} B
+${c_commit}COMMIT_ID${c_reset}${c_symbol} (${c_reset}\
+${c_tag}tag: ${c_reset}${c_tag}A1${c_reset}${c_symbol}, ${c_reset}\
+${c_remoteBranch}other/main${c_reset}${c_symbol})${c_reset} A1
+	${c_commit}COMMIT_ID${c_reset}${c_symbol} (${c_reset}\
+${c_stash}refs/stash${c_reset}${c_symbol})${c_reset} On main: Changes to A.t
+	${c_commit}COMMIT_ID${c_reset}${c_symbol} (${c_reset}\
+${c_tag}tag: ${c_reset}${c_tag}A${c_reset}${c_symbol})${c_reset} A
 	EOF
 
 	git log --first-parent --no-abbrev --decorate --oneline --color=always --all >actual &&
@@ -78,14 +80,14 @@ test_expect_success 'test coloring with replace-objects' '
 	git replace HEAD~1 HEAD~2 &&
 
 	cat >expect <<-EOF &&
-	${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_HEAD}HEAD${c_reset}\
-${c_commit} -> ${c_reset}${c_branch}main${c_reset}${c_commit}, \
-${c_reset}${c_tag}tag: ${c_reset}${c_tag}D${c_reset}${c_commit})${c_reset} D
-	${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}\
-${c_tag}tag: ${c_reset}${c_tag}C${c_reset}${c_commit}, \
-${c_reset}${c_grafted}replaced${c_reset}${c_commit})${c_reset} B
-	${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}\
-${c_tag}tag: ${c_reset}${c_tag}A${c_reset}${c_commit})${c_reset} A
+	${c_commit}COMMIT_ID${c_reset}${c_symbol} (${c_reset}${c_HEAD}HEAD${c_reset}\
+${c_symbol} -> ${c_reset}${c_branch}main${c_reset}${c_symbol}, ${c_reset}\
+${c_tag}tag: ${c_reset}${c_tag}D${c_reset}${c_symbol})${c_reset} D
+	${c_commit}COMMIT_ID${c_reset}${c_symbol} (${c_reset}\
+${c_tag}tag: ${c_reset}${c_tag}C${c_reset}${c_symbol}, ${c_reset}\
+${c_grafted}replaced${c_reset}${c_symbol})${c_reset} B
+	${c_commit}COMMIT_ID${c_reset}${c_symbol} (${c_reset}\
+${c_tag}tag: ${c_reset}${c_tag}A${c_reset}${c_symbol})${c_reset} A
 EOF
 
 	git log --first-parent --no-abbrev --decorate --oneline --color=always HEAD >actual &&
@@ -104,15 +106,15 @@ test_expect_success 'test coloring with grafted commit' '
 	git replace --graft HEAD HEAD~2 &&
 
 	cat >expect <<-EOF &&
-	${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_HEAD}HEAD${c_reset}\
-${c_commit} -> ${c_reset}${c_branch}main${c_reset}${c_commit}, \
-${c_reset}${c_tag}tag: ${c_reset}${c_tag}D${c_reset}${c_commit}, \
-${c_reset}${c_grafted}replaced${c_reset}${c_commit})${c_reset} D
-	${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}\
-${c_tag}tag: ${c_reset}${c_tag}v1.0${c_reset}${c_commit}, \
-${c_reset}${c_tag}tag: ${c_reset}${c_tag}B${c_reset}${c_commit})${c_reset} B
-	${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}\
-${c_tag}tag: ${c_reset}${c_tag}A${c_reset}${c_commit})${c_reset} A
+	${c_commit}COMMIT_ID${c_reset}${c_symbol} (${c_reset}${c_HEAD}HEAD${c_reset}\
+${c_symbol} -> ${c_reset}${c_branch}main${c_reset}${c_symbol}, ${c_reset}\
+${c_tag}tag: ${c_reset}${c_tag}D${c_reset}${c_symbol}, ${c_reset}\
+${c_grafted}replaced${c_reset}${c_symbol})${c_reset} D
+	${c_commit}COMMIT_ID${c_reset}${c_symbol} (${c_reset}\
+${c_tag}tag: ${c_reset}${c_tag}v1.0${c_reset}${c_symbol}, ${c_reset}\
+${c_tag}tag: ${c_reset}${c_tag}B${c_reset}${c_symbol})${c_reset} B
+	${c_commit}COMMIT_ID${c_reset}${c_symbol} (${c_reset}\
+${c_tag}tag: ${c_reset}${c_tag}A${c_reset}${c_symbol})${c_reset} A
 	EOF
 
 	git log --first-parent --no-abbrev --decorate --oneline --color=always HEAD >actual &&
-- 
2.42.GIT


^ permalink raw reply related

* [PATCH v2 3/6] log: add color.decorate.ref config variable
From: Andy Koppe @ 2023-10-22 21:44 UTC (permalink / raw)
  To: git; +Cc: gitster, Andy Koppe
In-Reply-To: <20231022214432.56325-1-andy.koppe@gmail.com>

Refs other than branches, remote-tracking branches, tags and the stash
do not appear in log decorations by default, but they can be shown by
using decoration filter options such as --clear-decorations or
log.initialDecorationSet. However, they would appear without color.

Add config variable color.decorate.ref for such refs, defaulting to bold
magenta, which is the same as refs/stash.

Document the new variable on the git-config page and amend
t4207-log-decoration-colors.sh to test it.

Signed-off-by: Andy Koppe <andy.koppe@gmail.com>
---
 Documentation/config/color.txt   | 5 +++++
 commit.h                         | 1 +
 log-tree.c                       | 4 +++-
 t/t4207-log-decoration-colors.sh | 9 +++++++--
 4 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/Documentation/config/color.txt b/Documentation/config/color.txt
index cc0a881125..005a2bdb03 100644
--- a/Documentation/config/color.txt
+++ b/Documentation/config/color.txt
@@ -90,11 +90,16 @@ color.decorate.<slot>::
 	lightweight and annotated tags
 `stash`;;
 	the stash ref
+`ref`;;
+	any other refs (not shown by default)
 `grafted`;;
 	grafted and replaced commits
 `symbol`;;
 	punctuation symbols surrounding the other elements
 --
++
+(Variable `log.initialDecorationSet` or linkgit:git-log[1] option
+`--clear-decorations` can be used to show all refs.)
 
 color.grep::
 	When set to `always`, always highlight matches.  When `false` (or
diff --git a/commit.h b/commit.h
index cb13e4d5ba..f6b2125fc4 100644
--- a/commit.h
+++ b/commit.h
@@ -54,6 +54,7 @@ enum decoration_type {
 	DECORATION_REF_REMOTE,
 	DECORATION_REF_TAG,
 	DECORATION_REF_STASH,
+	DECORATION_REF,
 	DECORATION_REF_HEAD,
 	DECORATION_GRAFTED,
 	DECORATION_SYMBOL,
diff --git a/log-tree.c b/log-tree.c
index 5ad168458e..36558f3008 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -39,6 +39,7 @@ static char decoration_colors[][COLOR_MAXLEN] = {
 	GIT_COLOR_BOLD_RED,	/* REF_REMOTE */
 	GIT_COLOR_BOLD_YELLOW,	/* REF_TAG */
 	GIT_COLOR_BOLD_MAGENTA,	/* REF_STASH */
+	GIT_COLOR_BOLD_MAGENTA, /* REF */
 	GIT_COLOR_BOLD_CYAN,	/* REF_HEAD */
 	GIT_COLOR_BOLD_BLUE,	/* GRAFTED */
 	GIT_COLOR_NIL,		/* SYMBOL */
@@ -49,6 +50,7 @@ static const char *color_decorate_slots[] = {
 	[DECORATION_REF_REMOTE] = "remoteBranch",
 	[DECORATION_REF_TAG]	= "tag",
 	[DECORATION_REF_STASH]	= "stash",
+	[DECORATION_REF]	= "ref",
 	[DECORATION_REF_HEAD]	= "HEAD",
 	[DECORATION_GRAFTED]	= "grafted",
 	[DECORATION_SYMBOL]	= "symbol",
@@ -151,7 +153,7 @@ static int add_ref_decoration(const char *refname, const struct object_id *oid,
 	int i;
 	struct object *obj;
 	enum object_type objtype;
-	enum decoration_type deco_type = DECORATION_NONE;
+	enum decoration_type deco_type = DECORATION_REF;
 	struct decoration_filter *filter = (struct decoration_filter *)cb_data;
 	const char *git_replace_ref_base = ref_namespace[NAMESPACE_REPLACE].ref;
 
diff --git a/t/t4207-log-decoration-colors.sh b/t/t4207-log-decoration-colors.sh
index f4173b6114..4b51e34f8b 100755
--- a/t/t4207-log-decoration-colors.sh
+++ b/t/t4207-log-decoration-colors.sh
@@ -17,6 +17,7 @@ test_expect_success setup '
 	git config color.decorate.remoteBranch red &&
 	git config color.decorate.tag "reverse bold yellow" &&
 	git config color.decorate.stash magenta &&
+	git config color.decorate.ref blue &&
 	git config color.decorate.grafted black &&
 	git config color.decorate.symbol white &&
 	git config color.decorate.HEAD cyan &&
@@ -28,11 +29,13 @@ test_expect_success setup '
 	c_remoteBranch="<RED>" &&
 	c_tag="<BOLD;REVERSE;YELLOW>" &&
 	c_stash="<MAGENTA>" &&
+	c_ref="<BLUE>" &&
 	c_HEAD="<CYAN>" &&
 	c_grafted="<BLACK>" &&
 	c_symbol="<WHITE>" &&
 
 	test_commit A &&
+	git update-ref refs/foo A &&
 	git clone . other &&
 	(
 		cd other &&
@@ -65,10 +68,12 @@ ${c_remoteBranch}other/main${c_reset}${c_symbol})${c_reset} A1
 	${c_commit}COMMIT_ID${c_reset}${c_symbol} (${c_reset}\
 ${c_stash}refs/stash${c_reset}${c_symbol})${c_reset} On main: Changes to A.t
 	${c_commit}COMMIT_ID${c_reset}${c_symbol} (${c_reset}\
-${c_tag}tag: ${c_reset}${c_tag}A${c_reset}${c_symbol})${c_reset} A
+${c_tag}tag: ${c_reset}${c_tag}A${c_reset}${c_symbol}, ${c_reset}\
+${c_ref}refs/foo${c_reset}${c_symbol})${c_reset} A
 	EOF
 
-	git log --first-parent --no-abbrev --decorate --oneline --color=always --all >actual &&
+	git log --first-parent --no-abbrev --decorate --clear-decorations \
+		--oneline --color=always --all >actual &&
 	cmp_filtered_decorations
 '
 
-- 
2.42.GIT


^ permalink raw reply related

* [PATCH v2 4/6] refs: add pseudorefs array and iteration functions
From: Andy Koppe @ 2023-10-22 21:44 UTC (permalink / raw)
  To: git; +Cc: gitster, Andy Koppe
In-Reply-To: <20231022214432.56325-1-andy.koppe@gmail.com>

Define const array 'pseudorefs' with the names of the pseudorefs that
are documented in gitrevisions.1, and add functions for_each_pseudoref()
and refs_for_each_pseudoref() for iterating over them.

The functions process the pseudorefs in the same way as head_ref() and
refs_head_ref() process HEAD, invoking an each_ref_fn callback on each
pseudoref that exists.

This is in preparation for adding pseudorefs to log decorations.

Signed-off-by: Andy Koppe <andy.koppe@gmail.com>
---
 refs.c | 42 ++++++++++++++++++++++++++++++++++++++++++
 refs.h |  5 +++++
 2 files changed, 47 insertions(+)

diff --git a/refs.c b/refs.c
index fcae5dddc6..aa7e4c02c5 100644
--- a/refs.c
+++ b/refs.c
@@ -65,6 +65,21 @@ static unsigned char refname_disposition[256] = {
 	0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 3, 0, 0, 4, 4
 };
 
+/*
+ * List of documented pseudorefs. This needs to be kept in sync with the list
+ * in Documentation/revisions.txt.
+ */
+static const char *const pseudorefs[] = {
+	"FETCH_HEAD",
+	"ORIG_HEAD",
+	"MERGE_HEAD",
+	"REBASE_HEAD",
+	"CHERRY_PICK_HEAD",
+	"REVERT_HEAD",
+	"BISECT_HEAD",
+	"AUTO_MERGE",
+};
+
 struct ref_namespace_info ref_namespace[] = {
 	[NAMESPACE_HEAD] = {
 		.ref = "HEAD",
@@ -1549,6 +1564,33 @@ int head_ref(each_ref_fn fn, void *cb_data)
 	return refs_head_ref(get_main_ref_store(the_repository), fn, cb_data);
 }
 
+int refs_for_each_pseudoref(struct ref_store *refs,
+			    each_ref_fn fn, void *cb_data)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(pseudorefs); i++) {
+		struct object_id oid;
+		int flag;
+
+		if (refs_resolve_ref_unsafe(refs, pseudorefs[i],
+					    RESOLVE_REF_READING, &oid, &flag)) {
+			int ret = fn(pseudorefs[i], &oid, flag, cb_data);
+
+			if (ret)
+				return ret;
+		}
+	}
+
+	return 0;
+}
+
+int for_each_pseudoref(each_ref_fn fn, void *cb_data)
+{
+	return refs_for_each_pseudoref(get_main_ref_store(the_repository),
+				       fn, cb_data);
+}
+
 struct ref_iterator *refs_ref_iterator_begin(
 		struct ref_store *refs,
 		const char *prefix,
diff --git a/refs.h b/refs.h
index 23211a5ea1..7b55cced31 100644
--- a/refs.h
+++ b/refs.h
@@ -320,6 +320,8 @@ typedef int each_repo_ref_fn(struct repository *r,
  */
 int refs_head_ref(struct ref_store *refs,
 		  each_ref_fn fn, void *cb_data);
+int refs_for_each_pseudoref(struct ref_store *refs,
+			    each_ref_fn fn, void *cb_data);
 int refs_for_each_ref(struct ref_store *refs,
 		      each_ref_fn fn, void *cb_data);
 int refs_for_each_ref_in(struct ref_store *refs, const char *prefix,
@@ -334,6 +336,9 @@ int refs_for_each_remote_ref(struct ref_store *refs,
 /* just iterates the head ref. */
 int head_ref(each_ref_fn fn, void *cb_data);
 
+/* iterates pseudorefs. */
+int for_each_pseudoref(each_ref_fn fn, void *cb_data);
+
 /* iterates all refs. */
 int for_each_ref(each_ref_fn fn, void *cb_data);
 
-- 
2.42.GIT


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox