* [PATCH v3 3/7] log: add color.decorate.symbol config variable
From: Andy Koppe @ 2023-10-23 22:11 UTC (permalink / raw)
To: git; +Cc: gitster, stolee, Andy Koppe
In-Reply-To: <20231023221143.72489-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 8bdf889f02..890024f205 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -41,6 +41,7 @@ static char decoration_colors[][COLOR_MAXLEN] = {
[DECORATION_REF_STASH] = GIT_COLOR_BOLD_MAGENTA,
[DECORATION_REF_HEAD] = GIT_COLOR_BOLD_CYAN,
[DECORATION_GRAFTED] = GIT_COLOR_BOLD_BLUE,
+ [DECORATION_SYMBOL] = GIT_COLOR_NIL,
};
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 v3 5/7] refs: add pseudorefs array and iteration functions
From: Andy Koppe @ 2023-10-23 22:11 UTC (permalink / raw)
To: git; +Cc: gitster, stolee, Andy Koppe
In-Reply-To: <20231023221143.72489-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
* [PATCH v3 4/7] log: add color.decorate.ref config variable
From: Andy Koppe @ 2023-10-23 22:11 UTC (permalink / raw)
To: git; +Cc: gitster, stolee, Andy Koppe
In-Reply-To: <20231023221143.72489-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 890024f205..fb3d87b83d 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -39,6 +39,7 @@ static char decoration_colors[][COLOR_MAXLEN] = {
[DECORATION_REF_REMOTE] = GIT_COLOR_BOLD_RED,
[DECORATION_REF_TAG] = GIT_COLOR_BOLD_YELLOW,
[DECORATION_REF_STASH] = GIT_COLOR_BOLD_MAGENTA,
+ [DECORATION_REF] = GIT_COLOR_BOLD_MAGENTA,
[DECORATION_REF_HEAD] = GIT_COLOR_BOLD_CYAN,
[DECORATION_GRAFTED] = GIT_COLOR_BOLD_BLUE,
[DECORATION_SYMBOL] = GIT_COLOR_NIL,
@@ -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 v3 2/7] log: use designated inits for decoration_colors
From: Andy Koppe @ 2023-10-23 22:11 UTC (permalink / raw)
To: git; +Cc: gitster, stolee, Andy Koppe
In-Reply-To: <20231023221143.72489-1-andy.koppe@gmail.com>
Use designated initializers instead of comments to denote the slots in
the decoration_colors array for holding color settings, to make it
consistent with the immediately following color_decorate_slots array
and reduce the likelihood of mistakes when extending them.
Signed-off-by: Andy Koppe <andy.koppe@gmail.com>
---
log-tree.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/log-tree.c b/log-tree.c
index 504da6b519..8bdf889f02 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -34,13 +34,13 @@ static int decoration_loaded;
static int decoration_flags;
static char decoration_colors[][COLOR_MAXLEN] = {
- GIT_COLOR_RESET,
- GIT_COLOR_BOLD_GREEN, /* REF_LOCAL */
- GIT_COLOR_BOLD_RED, /* REF_REMOTE */
- GIT_COLOR_BOLD_YELLOW, /* REF_TAG */
- GIT_COLOR_BOLD_MAGENTA, /* REF_STASH */
- GIT_COLOR_BOLD_CYAN, /* REF_HEAD */
- GIT_COLOR_BOLD_BLUE, /* GRAFTED */
+ [DECORATION_NONE] = GIT_COLOR_RESET,
+ [DECORATION_REF_LOCAL] = GIT_COLOR_BOLD_GREEN,
+ [DECORATION_REF_REMOTE] = GIT_COLOR_BOLD_RED,
+ [DECORATION_REF_TAG] = GIT_COLOR_BOLD_YELLOW,
+ [DECORATION_REF_STASH] = GIT_COLOR_BOLD_MAGENTA,
+ [DECORATION_REF_HEAD] = GIT_COLOR_BOLD_CYAN,
+ [DECORATION_GRAFTED] = GIT_COLOR_BOLD_BLUE,
};
static const char *color_decorate_slots[] = {
--
2.42.GIT
^ permalink raw reply related
* [PATCH v3 1/7] config: restructure color.decorate documentation
From: Andy Koppe @ 2023-10-23 22:11 UTC (permalink / raw)
To: git; +Cc: gitster, stolee, Andy Koppe
In-Reply-To: <20231023221143.72489-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 v3 0/7] log: decorate pseudorefs and other refs
From: Andy Koppe @ 2023-10-23 22:11 UTC (permalink / raw)
To: git; +Cc: gitster, stolee, Andy Koppe
In-Reply-To: <20231022214432.56325-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/6618230811
Andy Koppe (7):
config: restructure color.decorate documentation
log: use designated inits for decoration_colors
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 | 57 ++++++++++----
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, 192 insertions(+), 67 deletions(-)
--
2.42.GIT
^ permalink raw reply
* Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands
From: Dragan Simic @ 2023-10-23 21:26 UTC (permalink / raw)
To: Jacob Stopak; +Cc: Oswald Buddenhagen, Junio C Hamano, git
In-Reply-To: <ZTbkUUbmp56fgc+a.jacob@initialcommit.io>
On 2023-10-23 23:23, Jacob Stopak wrote:
> On Mon, Oct 23, 2023 at 10:59:08PM +0200, Dragan Simic wrote:
>> > On Mon, Oct 23, 2023 at 09:04:49PM +0200, Dragan Simic wrote:
>> > > That's exactly why I already wrote that the original author of
>> > > the table patches, if those would become accepted, should commit
>> > > in advance to maintaining that as the new git feature.
>> > >
>> > that's seems just a wee bit unfair (the bar isn't put that high for
>> > other features), and it's not realistically enforcable anyway.
>>
>> Well, it's a bit disruptive new feature, which is also quite extensive
>> and
>> can rather easily become obsolete if not maintained in the long run.
>> Perhaps we should hear Jacob's thoughts about that as well.
>
> I would love to work on implementing this and maintaining it! I spend
> too much time doing Git things for no reason anyway so why not? :D
Awesome! Quite frankly, I expected to hear that. :)
> But... I could die at the hands of an angry user and then you would
> have
> to take over, Dragan...
Or I could instead try to help you fending off angry git users? :)
^ permalink raw reply
* Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands
From: Jacob Stopak @ 2023-10-23 21:23 UTC (permalink / raw)
To: Dragan Simic; +Cc: Oswald Buddenhagen, Junio C Hamano, git
In-Reply-To: <c8e437eacdb883f612ae44e43c95602f@manjaro.org>
On Mon, Oct 23, 2023 at 10:59:08PM +0200, Dragan Simic wrote:
> > On Mon, Oct 23, 2023 at 09:04:49PM +0200, Dragan Simic wrote:
> > > That's exactly why I already wrote that the original author of
> > > the table patches, if those would become accepted, should commit
> > > in advance to maintaining that as the new git feature.
> > >
> > that's seems just a wee bit unfair (the bar isn't put that high for
> > other features), and it's not realistically enforcable anyway.
>
> Well, it's a bit disruptive new feature, which is also quite extensive and
> can rather easily become obsolete if not maintained in the long run.
> Perhaps we should hear Jacob's thoughts about that as well.
I would love to work on implementing this and maintaining it! I spend
too much time doing Git things for no reason anyway so why not? :D
But... I could die at the hands of an angry user and then you would have
to take over, Dragan...
^ permalink raw reply
* Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands
From: Dragan Simic @ 2023-10-23 21:19 UTC (permalink / raw)
To: Oswald Buddenhagen; +Cc: Jacob Stopak, git
In-Reply-To: <ZTbiU+SwvGi9972S@ugly>
On 2023-10-23 23:14, Oswald Buddenhagen wrote:
> On Mon, Oct 23, 2023 at 10:51:31PM +0200, Dragan Simic wrote:
>> The hint should be about how to turn the tables and verbosity on, not
>> how to get rid of it.
>
> but that's just backwards. a noob shouldn't be bothered with putting
> the tool into noob-friendly mode, neither actually nor "just"
> psychologically. switching to "expert" mode should be the conscious
> effort. and making it opt-in wouldn't even save the experts any
> annoyance, as they need to opt out from the hint anyway.
Let's focus on non-noobs as well, which could go nuts by having some
strange tables displayed after updating git. For the beginners, having
something configured is already inevitable, e.g. their credentials, so
they could also turn on the tables if they wanted so.
^ permalink raw reply
* Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands
From: Oswald Buddenhagen @ 2023-10-23 21:14 UTC (permalink / raw)
To: Dragan Simic; +Cc: Jacob Stopak, git
In-Reply-To: <d55eaf41f55bdff0b5ae734e5d7e6724@manjaro.org>
On Mon, Oct 23, 2023 at 10:51:31PM +0200, Dragan Simic wrote:
>The hint should be about how to turn the tables and verbosity on, not
>how to get rid of it.
>
but that's just backwards. a noob shouldn't be bothered with putting the
tool into noob-friendly mode, neither actually nor "just"
psychologically. switching to "expert" mode should be the conscious
effort. and making it opt-in wouldn't even save the experts any
annoyance, as they need to opt out from the hint anyway.
regards
^ permalink raw reply
* Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands
From: Jacob Stopak @ 2023-10-23 21:12 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Dragan Simic, Oswald Buddenhagen, git
In-Reply-To: <xmqqfs21noxx.fsf@gitster.g>
On Mon, Oct 23, 2023 at 12:01:14PM -0700, Junio C Hamano wrote:
> Ah, OK, now I see where your "--table" is coming from ;-).
> "git-sim" was exactly what I thought about when I saw it, and I did
> not know that "--table" came from the same set of brain cells.
Haha the set is not quite the same I'm sure I've lost many over the
course of this year. But the survivors are doing their best.
> One thing that nobody seems to have raised that disturbs me is that
> even though there may be educational value in having such a
> "feature", having to carry the extra code to implement in Git incurs
> extra cost. I was reasonably happy when I saw that "git-sim" was
> done as a totally separate entity exactly for this reason.
Erm, not to get too sappy here but I'd love to maintain anything related
to this that gets implemented, in whatever form that turns out to be.
I already spend way too much of my free time working on Git-related
things so making this contribution to Git itself would mean a lot to me.
Starting Git-Sim as a separate entity made sense to me because:
* I had no idea whether anyone wanted something like this, so it
would have made for a pretty weak argument to the community. (It
turned out way more users were interested than I thought).
* It's written in Python and relies on a dependency library called
Manim, so I thought it wouldn't make any sense to try and wedge
that into the Git codebase.
* The output is presentation quality images / video animations, which
is unlike anything I've seen outputted by any Git command. (I didn't
explore what it would take to do something similar in C).
The main downsides to Git-Sim as a separate tool are:
* The main downside is lack of reach. Not being Git-native means only
a tiny fraction of Git users will ever know it exists.
* There is technically no guarantee that a simulated output actually
corresponds to what the command will do, as highlighted by this
comment on Hacker News: "Next HN post - "I destroyed my repo - but
it WORKED in Git-Sim!"
* As Git changes over time, Git-Sim is destined to be a step behind.
* Certain commands (like the networked commands fetch/push/pull/etc)
are not easy to simulate without doing horrible things like cloning
a new copy of the repo behind the scenes, running the desired
operation on it, and checking the result. I assume things like this
would be a lot easier to do within Git.
^ permalink raw reply
* Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands
From: Dragan Simic @ 2023-10-23 20:59 UTC (permalink / raw)
To: Oswald Buddenhagen; +Cc: Junio C Hamano, Jacob Stopak, git
In-Reply-To: <ZTbb7bHkFOOyBT6+@ugly>
> On Mon, Oct 23, 2023 at 09:04:49PM +0200, Dragan Simic wrote:
>> That's exactly why I already wrote that the original author of
>> the table patches, if those would become accepted, should commit
>> in advance to maintaining that as the new git feature.
>>
> that's seems just a wee bit unfair (the bar isn't put that high for
> other features), and it's not realistically enforcable anyway.
Well, it's a bit disruptive new feature, which is also quite extensive
and can rather easily become obsolete if not maintained in the long run.
Perhaps we should hear Jacob's thoughts about that as well.
^ permalink raw reply
* Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands
From: Dragan Simic @ 2023-10-23 20:51 UTC (permalink / raw)
To: Oswald Buddenhagen; +Cc: Jacob Stopak, git
In-Reply-To: <ZTbVY7Nf+DTYqHky@ugly>
On 2023-10-23 22:19, Oswald Buddenhagen wrote:
> On Mon, Oct 23, 2023 at 12:29:12PM -0700, Jacob Stopak wrote:
>> Those arrows showing how things move only really apply to "simulating"
>> (dry runs) for specific commands like add, restore, rm, commit, stash,
>> etc, so making the --table proposal a default status output would
>> still
>> miss those scenarios.
>>
> you're too focused on the status quo of your own tool. :-)
> there is really nothing that would speak against the real commands
> reporting what they just *actually did*. this would seem rather
> helpful for noobs and other insecure users.
> if one really wanted, "you can also use this with --dry-run" could be
> part of the hint that would say how to turn off the extra verbosity
> (or just the hint itself, if one likes the verbosity).
The hint should be about how to turn the tables and verbosity on, not
how to get rid of it.
> one could even go one step further and put at least the destructive
> commands into interactive/confirmation mode by default. but that's
> probably a bridge too far, as it would be potentially habit-forming in
> a bad way.
>
> regards
^ permalink raw reply
* Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands
From: Oswald Buddenhagen @ 2023-10-23 20:47 UTC (permalink / raw)
To: Dragan Simic; +Cc: Junio C Hamano, Jacob Stopak, git
In-Reply-To: <18c7b1bea07d5f3878f4466b8e133da1@manjaro.org>
>On 2023-10-23 21:01, Junio C Hamano wrote:
>> One thing that nobody seems to have raised that disturbs me is that
>> even though there may be educational value in having such a
>> "feature", having to carry the extra code to implement in Git incurs
>> extra cost.
>
that's the case for literally every feature. you're just noticing it
here, because from your expert perspective it seems redundant with
pre-existing functionality. so assuming that you actually care about
newbie UX, you are at least not convinced that it would be a significant
help.
i'm not totally convinced either, which is why i wrote of "valid
experiment" upthread. but i don't think that measuring the actual impact
is all that interesting unless the feature turns out to be actually
excessively costly in the long run.
On Mon, Oct 23, 2023 at 09:04:49PM +0200, Dragan Simic wrote:
>That's exactly why I already wrote that the original author of the table
>patches, if those would become accepted, should commit in advance to
>maintaining that as the new git feature.
>
that's seems just a wee bit unfair (the bar isn't put that high for
other features), and it's not realistically enforcable anyway.
regards
^ permalink raw reply
* Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands
From: Dragan Simic @ 2023-10-23 20:29 UTC (permalink / raw)
To: Jacob Stopak; +Cc: Oswald Buddenhagen, git
In-Reply-To: <ZTbJiIkIyXwWK8JP.jacob@initialcommit.io>
On 2023-10-23 21:29, Jacob Stopak wrote:
> However, now that I'm thinking about it maybe it could somehow be
> included
> in the Hints feature? I honestly don't know exactly when the hints are
> currently invoked or how much detail they go into, but what just popped
> into my head is kindof a "universal dry run" option, which would show
> the
> user the --table format hint when they invoke an applicable command,
> and
> prompt them if they actually want to run it.
That's a good idea, having the tables and dry runs mentioned in a
well-placed hint that could, of course, be disabled through git
configuration.
^ permalink raw reply
* Re: [PATCH v4 00/17] bloom: changed-path Bloom filters v2 (& sundries)
From: SZEDER Gábor @ 2023-10-23 20:22 UTC (permalink / raw)
To: Taylor Blau
Cc: Junio C Hamano, git, Elijah Newren, Eric W. Biederman, Jeff King,
Patrick Steinhardt
In-Reply-To: <ZTK4ZKESDVghzSH8@nand.local>
On Fri, Oct 20, 2023 at 01:27:00PM -0400, Taylor Blau wrote:
> On Wed, Oct 18, 2023 at 04:26:48PM -0700, Junio C Hamano wrote:
> > Taylor Blau <me@ttaylorr.com> writes:
> >
> > > (Rebased onto the tip of 'master', which is 3a06386e31 (The fifteenth
> > > batch, 2023-10-04), at the time of writing).
> >
> > Judging from 17/17 that has a free_commit_graph() call in
> > close_commit_graph(), that was merged in the eighteenth batch,
> > the above is probably untrue. I'll apply to the current master and
> > see how it goes instead.
>
> Worse than that, I sent this `--in-reply-to` the wrong thread :-<.
>
> Sorry about that, and indeed you are right that the correct base for
> this round should be a9ecda2788 (The eighteenth batch, 2023-10-13).
>
> I'm optimistic that with the amount of careful review that this topic
> has already received, that this round should do the trick.
Unfortunately, I can't share this optimism. This series still lacks
tests exercising the interaction of different versions of Bloom
filters and split commit graphs, and the one such test that I sent a
while ago demonstrates that it's still broken. And it's getting
worse: back then I didn't send the related test that merged
commit-graph layers containing different Bloom filter versions,
because happened to succeed even back then; but, alas, with this
series even that test fails.
^ permalink raw reply
* Re: [PATCH v3] builtin/branch.c: adjust error messages to coding guidelines
From: Junio C Hamano @ 2023-10-23 20:19 UTC (permalink / raw)
To: Rubén Justo; +Cc: Isoken June Ibizugbe, git, Dragan Simic
In-Reply-To: <14df596a-d0ef-46aa-97c6-3c9f0da1975f@gmail.com>
Rubén Justo <rjusto@gmail.com> writes:
> The rest of the patch is equal to the previous iteration (v2), which
> already seemed correct to me.
>
> Thank you.
Thanks, all. Let's declare a victory and mark the topic to be
merged to 'next'.
^ permalink raw reply
* Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands
From: Oswald Buddenhagen @ 2023-10-23 20:19 UTC (permalink / raw)
To: Jacob Stopak; +Cc: Dragan Simic, git
In-Reply-To: <ZTbJiIkIyXwWK8JP.jacob@initialcommit.io>
On Mon, Oct 23, 2023 at 12:29:12PM -0700, Jacob Stopak wrote:
>Those arrows showing how things move only really apply to "simulating"
>(dry runs) for specific commands like add, restore, rm, commit, stash,
>etc, so making the --table proposal a default status output would still
>miss those scenarios.
>
you're too focused on the status quo of your own tool. :-)
there is really nothing that would speak against the real commands
reporting what they just *actually did*. this would seem rather helpful
for noobs and other insecure users.
if one really wanted, "you can also use this with --dry-run" could be
part of the hint that would say how to turn off the extra verbosity (or
just the hint itself, if one likes the verbosity).
one could even go one step further and put at least the destructive
commands into interactive/confirmation mode by default. but that's
probably a bridge too far, as it would be potentially habit-forming in a
bad way.
regards
^ permalink raw reply
* Re: [PATCH v3] builtin/branch.c: adjust error messages to coding guidelines
From: Rubén Justo @ 2023-10-23 20:17 UTC (permalink / raw)
To: Isoken June Ibizugbe, git; +Cc: Dragan Simic, Junio C Hamano
In-Reply-To: <20231023160656.4341-1-isokenjune@gmail.com>
On 23-oct-2023 17:06:56, Isoken June Ibizugbe wrote:
Just for reference, to avoid confusion, this is technically the fourth
version.
> As per the CodingGuidelines document, it is recommended that error messages
> such as die(), error() and warning(), should start with a lowercase letter
> and should not end with a period.
>
> This patch adjusts tests to match updated messages.
>
> Signed-off-by: Isoken June Ibizugbe <isokenjune@gmail.com>
> ---
It is often convenient to include here, after this three dash line, a
description of the changes introduced in the new iteration.
In this particular case, a range-diff is very helpful.
This is the range-diff with v2:
1: a4e8bb1b4c ! 1: c4ae0c1cce builtin/branch.c: adjust error messages to coding guidelines
@@ Metadata
## Commit message ##
builtin/branch.c: adjust error messages to coding guidelines
- As per the CodingGuidelines document, it is recommended that a single-line
- message provided to error messages such as die(), error() and warning(),
- should start with a lowercase letter and should not end with a period.
- Also this patch fixes the tests broken by the changes.
+ As per the CodingGuidelines document, it is recommended that error messages
+ such as die(), error() and warning(), should start with a lowercase letter
+ and should not end with a period.
+
+ This patch adjusts tests to match updated messages.
Signed-off-by: Isoken June Ibizugbe <isokenjune@gmail.com>
As we can see, the only difference is in the commit message.
The new wording is better; it avoids the distraction introduced in v2.
And, as Dragan suggested about /the other v3/, the wrapping is correct.
The rest of the patch is equal to the previous iteration (v2), which
already seemed correct to me.
Thank you.
^ permalink raw reply
* Re: [PATCH v3 0/5] config-parse: create config parsing library
From: Junio C Hamano @ 2023-10-23 20:13 UTC (permalink / raw)
To: Taylor Blau; +Cc: Josh Steadmon, git, jonathantanmy, calvinwan, glencbz
In-Reply-To: <ZTbK3QTJYXxYj/M6@nand.local>
Taylor Blau <me@ttaylorr.com> writes:
> This feels like churn that does not yield a tangible pay-off, at least
> in the sense that the refactoring and code movement delivers us
> something that we can substantively use today.
>
> I dunno.
That matches something I felt but was too polite to say aloud ;-)
^ permalink raw reply
* Re: [PATCH v3 3/5] config: report config parse errors using cb
From: Junio C Hamano @ 2023-10-23 20:11 UTC (permalink / raw)
To: Taylor Blau; +Cc: Josh Steadmon, git, jonathantanmy, calvinwan, glencbz
In-Reply-To: <ZTbJqzWDyqkhc6L9@nand.local>
Taylor Blau <me@ttaylorr.com> writes:
>> + struct config_parse_options opts = CP_OPTS_INIT(CONFIG_ERROR_ERROR);
>
> I'm nit-picking, but I find this parameterized initializer macro to be a
> little unusual w.r.t our usual conventions.
>
> In terms of "usual conventions," I'm thinking about STRING_LIST_INIT_DUP
> versus STRING_LIST_INIT_NODUP (as opposed to something like
> STRING_LIST_INIT(DUP) or STRING_LIST_INIT(NODUP)).
FWIW, I have always felt that the way STRING_LIST_INIT* was done was
quite ugly. The new pattern does look superiour, as long as (1) it
does not involve voodoo like token pasting, and (2) the parameters
passed does not grow. The latter is especially important as there
is no equivalent to designated initializers in C preprocessor macros.
Thanks.
^ permalink raw reply
* Re: [PATCH 2/3] Revert "send-email: extract email-parsing code into a subroutine"
From: Oswald Buddenhagen @ 2023-10-23 19:50 UTC (permalink / raw)
To: Jeff King
Cc: Michael Strawbridge, Junio C Hamano, Bagas Sanjaya,
Git Mailing List
In-Reply-To: <20231023184010.GA1537181@coredump.intra.peff.net>
On Mon, Oct 23, 2023 at 02:40:10PM -0400, Jeff King wrote:
>On Fri, Oct 20, 2023 at 12:45:43PM +0200, Oswald Buddenhagen wrote:
>> that seems like a rather significant point, doesn't it?
>
>Maybe. It depends on whether anybody is interested in adding
>continuation support. Nobody has in the previous 18 years, and nobody
>has asked for it.
>
dunno, it seems like a bug to me. so if i cared at all about this
functionality, i'd fix it just because. so at least it doesn't seem nice
to make it harder for a potential volunteer.
>> > So another option is to just fix the individual bugs separately.
>> >
>> ... so that seems preferable to me, given that the necessary fixes
>> seem
>> rather trivial.
>
>They're not too bad. Probably:
>
> 1. lc() the keys we put into the hash
>
> 2. match to/cc/bcc and dereference their arrays
>
> 3. maybe handle 'body' separately from headers to avoid confusion
>
with the header keys lowercased, one could simply use BODY as the key
and be done with it.
>But there may be other similar bugs lurking.
>One I didn't mention: the
>hash-based version randomly reorders headers!
>
hmm, yeah, that would mean using Tie::IxHash if one wanted to do it
elegantly, at the cost of depending on another non-core module.
also, it means that another hash with non-lowercased versions of the
keys would have to be kept.
ok, that's stupid. it would be easier to just keep an additional array
of the original keys for iteration, and check the hash before emitting
them.
>> > I guess "readable" is up for debate here, but I find the inline handling
>> > a lot easier to follow
>> >
>> any particular reason for that?
>
>For the reasons I gave in the commit message: namely that the matching
>and logic is in one place and doesn't need to be duplicated (e.g., the
>special handling of to/cc/bcc, which caused a bug here).
>
from what i can see, there isn't really anything to "match", apart from
agreeing on the data structure (which the code partially failed to do,
but that's trivial enough). and layering/abstracting things is usually
considered a good thing, unless the cost/benefit ratio is completely
backwards.
>The "//" one would
>work, but we support perl versions old enough that they don't have it.
>
according to my grepping, that ship has sailed.
also, why _would_ you support such ancient perl versions? that makes
even less sense to me than supporting ancient c compilers.
regards
^ permalink raw reply
* Re: [PATCH v2] doc/git-bisect: clarify `git bisect run` syntax
From: Eric Sunshine @ 2023-10-23 19:36 UTC (permalink / raw)
To: cousteau via GitGitGadget; +Cc: git, Patrick Steinhardt, Javier Mora
In-Reply-To: <pull.1602.v2.git.1698088990478.gitgitgadget@gmail.com>
On Mon, Oct 23, 2023 at 3:23 PM cousteau via GitGitGadget
<gitgitgadget@gmail.com> wrote:
> doc/git-bisect: clarify `git bisect run` syntax
>
> 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,
> in both the manpage and the `git bisect -h` command output.
>
> Additionally, change `--term-{new,bad}` et al to `--term-(new|bad)`
> for consistency with the synopsis syntax conventions.
Makes sense to fix this inconsistency, as well, though the patch
subject becomes a bit outdated with this addition.
> Signed-off-by: Javier Mora <cousteaulecommandant@gmail.com>
> ---
> diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
> @@ -16,7 +16,7 @@ DESCRIPTION
> - git bisect start [--term-{new,bad}=<term> --term-{old,good}=<term>]
> + git bisect start [--term-(new|bad)=<term-new> --term-(old|good)=<term-old>]
> [--no-checkout] [--first-parent] [<bad> [<good>...]] [--] [<paths>...]
> git bisect (bad|new|<term-new>) [<rev>]
> git bisect (good|old|<term-old>) [<rev>...]
Upon first reading, I questioned whether changing <term> to <term-new>
and <term-old> adds value since the option names --term-new and
--term-old already provide enough context for the reader to understand
the generic placeholder <term>. However, then I noticed that the
following two lines are already referencing placeholders <term-new>
and <term-old>, so perhaps this change makes sense. But...
> diff --git a/builtin/bisect.c b/builtin/bisect.c
> @@ -26,7 +26,7 @@ static GIT_PATH_FUNC(git_path_bisect_first_parent, "BISECT_FIRST_PARENT")
> #define BUILTIN_GIT_BISECT_START_USAGE \
> - N_("git bisect start [--term-{new,bad}=<term> --term-{old,good}=<term>]" \
> + N_("git bisect start [--term-(new|bad)=<term> --term-(old|good)=<term>]" \
...now we have an inconsistency again since this text just uses the
generic <term>. However, I haven't convinced myself that we need to
care about this inconsistency.
^ permalink raw reply
* Re: [PATCH v3 0/5] config-parse: create config parsing library
From: Taylor Blau @ 2023-10-23 19:34 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Josh Steadmon, git, jonathantanmy, calvinwan, glencbz
In-Reply-To: <xmqq34y9jho2.fsf@gitster.g>
On Tue, Oct 17, 2023 at 10:13:49AM -0700, Junio C Hamano wrote:
> Josh Steadmon <steadmon@google.com> writes:
>
> > Open questions:
> > - How do folks feel about the do_event() refactor in patches 2 & 3?
>
> I gave a quick re-read and found that the code after patch 2 made it
> easier to see how config.c::do_event() does its thing (even though
> the patch text of that exact step was somehow a bit hard to follow).
>
> However, the helper added by patch 3, do_event_and_flush(), that
> duplicates exactly what do_event() does, is hard to reason about, at
> least for me. It returns early without setting .previous_type to
> EOF and the value returned from the helper signals if that is the
> case (the two early return points both return what flush_event()
> gave us), but the only caller of the helper does not even inspect
> the return value, unlike all the callers of do_event(), which also
> looks a bit fishy.
I had similar thoughts while reviewing.
But I am not sure that I agree that this series is moving us in the
right direction necessarily. Or at least I am not convinced that
shipping the intermediate state is worth doing before we have callers
that could drop '#include "config.h"' for just the parser.
This feels like churn that does not yield a tangible pay-off, at least
in the sense that the refactoring and code movement delivers us
something that we can substantively use today.
I dunno.
Thanks,
Taylor
^ permalink raw reply
* Re: [PATCH v3 3/5] config: report config parse errors using cb
From: Taylor Blau @ 2023-10-23 19:29 UTC (permalink / raw)
To: Josh Steadmon; +Cc: git, jonathantanmy, calvinwan, glencbz, gitster
In-Reply-To: <a888045c04d27864edf5751ea8641fdba596779c.1695330852.git.steadmon@google.com>
On Thu, Sep 21, 2023 at 02:17:22PM -0700, Josh Steadmon wrote:
> diff --git a/bundle-uri.c b/bundle-uri.c
> index f93ca6a486..856bffdcad 100644
> --- a/bundle-uri.c
> +++ b/bundle-uri.c
> @@ -237,9 +237,7 @@ int bundle_uri_parse_config_format(const char *uri,
> struct bundle_list *list)
> {
> int result;
> - struct config_parse_options opts = {
> - .error_action = CONFIG_ERROR_ERROR,
> - };
> + struct config_parse_options opts = CP_OPTS_INIT(CONFIG_ERROR_ERROR);
I'm nit-picking, but I find this parameterized initializer macro to be a
little unusual w.r.t our usual conventions.
In terms of "usual conventions," I'm thinking about STRING_LIST_INIT_DUP
versus STRING_LIST_INIT_NODUP (as opposed to something like
STRING_LIST_INIT(DUP) or STRING_LIST_INIT(NODUP)).
Since there are only two possible values (the ones corresponding to
error() and die()) I wonder if something like CP_OPTS_INIT_ERROR and
CP_OPTS_INIT_DIE might be more appropriate. If you don't like either of
those, I'd suggest making the initializer a function instead of a
parameterized macro.
> if (!list->baseURI) {
> struct strbuf baseURI = STRBUF_INIT;
> diff --git a/config.c b/config.c
> index ff138500a2..0c4f1a2874 100644
> --- a/config.c
> +++ b/config.c
> @@ -55,7 +55,6 @@ struct config_source {
> enum config_origin_type origin_type;
> const char *name;
> const char *path;
> - enum config_error_action default_error_action;
> int linenr;
> int eof;
> size_t total_len;
> @@ -185,13 +184,15 @@ static int handle_path_include(const struct key_value_info *kvi,
> }
>
> if (!access_or_die(path, R_OK, 0)) {
> + struct config_parse_options config_opts = CP_OPTS_INIT(CONFIG_ERROR_DIE);
> +
> if (++inc->depth > MAX_INCLUDE_DEPTH)
> die(_(include_depth_advice), MAX_INCLUDE_DEPTH, path,
> !kvi ? "<unknown>" :
> kvi->filename ? kvi->filename :
> "the command line");
> ret = git_config_from_file_with_options(git_config_include, path, inc,
> - kvi->scope, NULL);
> + kvi->scope, &config_opts);
...OK, so using the CONFIG_ERROR_DIE variant seems like the right choice
here because git_config_from_file_with_options() calls
do_config_from_file() which sets its default_error_action as
CONFIG_ERROR_DIE.
> static uintmax_t get_unit_factor(const char *end)
> @@ -2023,7 +2052,6 @@ static int do_config_from_file(config_fn_t fn,
> top.origin_type = origin_type;
> top.name = name;
> top.path = path;
> - top.default_error_action = CONFIG_ERROR_DIE;
> top.do_fgetc = config_file_fgetc;
> top.do_ungetc = config_file_ungetc;
> top.do_ftell = config_file_ftell;
> @@ -2037,8 +2065,10 @@ static int do_config_from_file(config_fn_t fn,
> static int git_config_from_stdin(config_fn_t fn, void *data,
> enum config_scope scope)
> {
> + struct config_parse_options config_opts = CP_OPTS_INIT(CONFIG_ERROR_DIE);
> +
> return do_config_from_file(fn, CONFIG_ORIGIN_STDIN, "", NULL, stdin,
> - data, scope, NULL);
> + data, scope, &config_opts);
Same here.
> int git_config_from_file_with_options(config_fn_t fn, const char *filename,
> @@ -2061,8 +2091,10 @@ int git_config_from_file_with_options(config_fn_t fn, const char *filename,
>
> int git_config_from_file(config_fn_t fn, const char *filename, void *data)
> {
> + struct config_parse_options config_opts = CP_OPTS_INIT(CONFIG_ERROR_DIE);
> +
> return git_config_from_file_with_options(fn, filename, data,
> - CONFIG_SCOPE_UNKNOWN, NULL);
> + CONFIG_SCOPE_UNKNOWN, &config_opts);
> }
And here.
> @@ -2098,6 +2129,7 @@ int git_config_from_blob_oid(config_fn_t fn,
> char *buf;
> unsigned long size;
> int ret;
> + struct config_parse_options config_opts = CP_OPTS_INIT(CONFIG_ERROR_ERROR);
>
> buf = repo_read_object_file(repo, oid, &type, &size);
> if (!buf)
> @@ -2108,7 +2140,7 @@ int git_config_from_blob_oid(config_fn_t fn,
> }
>
> ret = git_config_from_mem(fn, CONFIG_ORIGIN_BLOB, name, buf, size,
> - data, scope, NULL);
> + data, scope, &config_opts);
> free(buf);
This one uses git_config_from_mem(), which sets the default error action
to "CONFIG_ERROR_ERROR", so this transformation looks correct.
Thanks,
Taylor
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox