Git development
 help / color / mirror / Atom feed
* Re: [PATCH 2/5] strbuf: factor out strbuf_expand_step()
From: René Scharfe @ 2023-06-27 16:21 UTC (permalink / raw)
  To: Jeff King; +Cc: Git List
In-Reply-To: <20230627082622.GH1226768@coredump.intra.peff.net>

Am 27.06.23 um 10:26 schrieb Jeff King:
> On Sat, Jun 17, 2023 at 10:41:44PM +0200, René Scharfe wrote:
>
>> Extract the part of strbuf_expand that finds the next placeholder into a
>> new function.  It allows to build parsers without callback functions and
>> the overhead imposed by them.
>>
>> Signed-off-by: René Scharfe <l.s.r@web.de>
>> ---
>>  builtin/branch.c | 13 ++-----------
>>  strbuf.c         | 28 ++++++++++++++--------------
>>  strbuf.h         |  8 ++++++++
>>  3 files changed, 24 insertions(+), 25 deletions(-)
>
> I was a little surprised to see the change in branch.c here (as well as
> the one in strbuf_addftime), since the commit message just says
> "extract..." and doesn't mention them.

Fair point, the other extraction sources should have been mentioned as
well somehow.

> They do serve as examples of how it can be used, so I think it's OK. But
> it made me wonder: is this all of the sites? Or are these just examples?

These are all that I could find.

René

^ permalink raw reply

* [PATCH v3 6/8] attr: expose and rename accessor functions
From: brian m. carlson @ 2023-06-27 16:19 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Junio C Hamano, Eric Sunshine, Derrick Stolee
In-Reply-To: <20230627161902.754472-1-sandals@crustytoothpaste.net>

From: "brian m. carlson" <bk2204@github.com>

Right now, the functions which determine the current system and global
gitattributes files are not exposed.  We'd like to use them in a future
commit, but they're not ideally named.  Rename them to something more
suitable as a public interface, expose them, and document them.

Signed-off-by: brian m. carlson <bk2204@github.com>
---
 attr.c | 14 +++++++-------
 attr.h |  9 +++++++++
 2 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/attr.c b/attr.c
index d45d34058d..66203ce322 100644
--- a/attr.c
+++ b/attr.c
@@ -870,7 +870,7 @@ static struct attr_stack *read_attr(struct index_state *istate,
 	return res;
 }
 
-static const char *git_etc_gitattributes(void)
+const char *git_attr_system_file(void)
 {
 	static const char *system_wide;
 	if (!system_wide)
@@ -878,7 +878,7 @@ static const char *git_etc_gitattributes(void)
 	return system_wide;
 }
 
-static const char *get_home_gitattributes(void)
+const char *git_attr_global_file(void)
 {
 	if (!git_attributes_file)
 		git_attributes_file = xdg_config_home("attributes");
@@ -886,7 +886,7 @@ static const char *get_home_gitattributes(void)
 	return git_attributes_file;
 }
 
-static int git_attr_system(void)
+int git_attr_system_is_enabled(void)
 {
 	return !git_env_bool("GIT_ATTR_NOSYSTEM", 0);
 }
@@ -920,14 +920,14 @@ static void bootstrap_attr_stack(struct index_state *istate,
 	push_stack(stack, e, NULL, 0);
 
 	/* system-wide frame */
-	if (git_attr_system()) {
-		e = read_attr_from_file(git_etc_gitattributes(), flags);
+	if (git_attr_system_is_enabled()) {
+		e = read_attr_from_file(git_attr_system_file(), flags);
 		push_stack(stack, e, NULL, 0);
 	}
 
 	/* home directory */
-	if (get_home_gitattributes()) {
-		e = read_attr_from_file(get_home_gitattributes(), flags);
+	if (git_attr_global_file()) {
+		e = read_attr_from_file(git_attr_global_file(), flags);
 		push_stack(stack, e, NULL, 0);
 	}
 
diff --git a/attr.h b/attr.h
index 676bd17ce2..2b745df405 100644
--- a/attr.h
+++ b/attr.h
@@ -227,4 +227,13 @@ void git_attr_set_direction(enum git_attr_direction new_direction);
 
 void attr_start(void);
 
+/* Return the system gitattributes file. */
+const char *git_attr_system_file(void);
+
+/* Return the global gitattributes file, if any. */
+const char *git_attr_global_file(void);
+
+/* Return whether the system gitattributes file is enabled and should be used. */
+int git_attr_system_is_enabled(void);
+
 #endif /* ATTR_H */

^ permalink raw reply related

* [PATCH v3 7/8] var: add attributes files locations
From: brian m. carlson @ 2023-06-27 16:19 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Junio C Hamano, Eric Sunshine, Derrick Stolee
In-Reply-To: <20230627161902.754472-1-sandals@crustytoothpaste.net>

From: "brian m. carlson" <bk2204@github.com>

Currently, there are some programs which would like to read and parse
the gitattributes files at the global or system levels.  However, it's
not always obvious where these files live, especially for the system
file, which may have been hard-coded at compile time or computed
dynamically based on the runtime prefix.

It's not reasonable to expect all callers of Git to intuitively know
where the Git distributor or user has configured these locations to
be, so add some entries to allow us to determine their location.  Honor
the GIT_ATTR_NOSYSTEM environment variable if one is specified.  Expose
the accessor functions in a way that we can reuse them from within the
var code.

In order to make our paths consistent on Windows and also use the same
form as paths use in "git rev-parse", let's normalize the path before we
return it.  This results in Windows-style paths that use slashes, which
is convenient for making our tests function in a consistent way across
platforms.  Note that this requires that some of our values be freed, so
let's add a flag about whether the value needs to be freed and use it
accordingly.

Signed-off-by: brian m. carlson <bk2204@github.com>
---
 Documentation/git-var.txt |  6 ++++++
 builtin/var.c             | 29 +++++++++++++++++++++++++++++
 t/t0007-git-var.sh        | 20 ++++++++++++++++++++
 3 files changed, 55 insertions(+)

diff --git a/Documentation/git-var.txt b/Documentation/git-var.txt
index f0f647e14a..dfbe5cb52b 100644
--- a/Documentation/git-var.txt
+++ b/Documentation/git-var.txt
@@ -74,6 +74,12 @@ GIT_DEFAULT_BRANCH::
 GIT_SHELL_PATH::
     The path of the binary providing the POSIX shell for commands which use the shell.
 
+GIT_ATTR_SYSTEM::
+    The path to the system linkgit:gitattributes[5] file, if one is enabled.
+
+GIT_ATTR_GLOBAL::
+    The path to the global (per-user) linkgit:gitattributes[5] file.
+
 SEE ALSO
 --------
 linkgit:git-commit-tree[1]
diff --git a/builtin/var.c b/builtin/var.c
index d6f9f495c9..79f7bdf55f 100644
--- a/builtin/var.c
+++ b/builtin/var.c
@@ -4,6 +4,7 @@
  * Copyright (C) Eric Biederman, 2005
  */
 #include "builtin.h"
+#include "attr.h"
 #include "config.h"
 #include "editor.h"
 #include "ident.h"
@@ -51,6 +52,26 @@ static char *shell_path(int ident_flag UNUSED)
 	return xstrdup(SHELL_PATH);
 }
 
+static char *git_attr_val_system(int ident_flag UNUSED)
+{
+	if (git_attr_system_is_enabled()) {
+		char *file = xstrdup(git_attr_system_file());
+		normalize_path_copy(file, file);
+		return file;
+	}
+	return NULL;
+}
+
+static char *git_attr_val_global(int ident_flag UNUSED)
+{
+	char *file = xstrdup(git_attr_global_file());
+	if (file) {
+		normalize_path_copy(file, file);
+		return file;
+	}
+	return NULL;
+}
+
 struct git_var {
 	const char *name;
 	char *(*read)(int);
@@ -84,6 +105,14 @@ static struct git_var git_vars[] = {
 		.name = "GIT_SHELL_PATH",
 		.read = shell_path,
 	},
+	{
+		.name = "GIT_ATTR_SYSTEM",
+		.read = git_attr_val_system,
+	},
+	{
+		.name = "GIT_ATTR_GLOBAL",
+		.read = git_attr_val_global,
+	},
 	{
 		.name = "",
 		.read = NULL,
diff --git a/t/t0007-git-var.sh b/t/t0007-git-var.sh
index e35f07afcb..374d9f49e9 100755
--- a/t/t0007-git-var.sh
+++ b/t/t0007-git-var.sh
@@ -162,6 +162,26 @@ test_expect_success MINGW 'GIT_SHELL_PATH points to a suitable shell' '
 	esac
 '
 
+test_expect_success 'GIT_ATTR_SYSTEM produces expected output' '
+	test_must_fail env GIT_ATTR_NOSYSTEM=1 git var GIT_ATTR_SYSTEM &&
+	(
+		sane_unset GIT_ATTR_NOSYSTEM &&
+		systempath=$(git var GIT_ATTR_SYSTEM) &&
+		test "$systempath" != ""
+	)
+'
+
+test_expect_success 'GIT_ATTR_GLOBAL points to the correct location' '
+	TRASHDIR="$(test-tool path-utils normalize_path_copy "$(pwd)")" &&
+	globalpath=$(XDG_CONFIG_HOME="$TRASHDIR/.config" git var GIT_ATTR_GLOBAL) &&
+	test "$globalpath" = "$TRASHDIR/.config/git/attributes" &&
+	(
+		sane_unset XDG_CONFIG_HOME &&
+		globalpath=$(HOME="$TRASHDIR" git var GIT_ATTR_GLOBAL) &&
+		test "$globalpath" = "$TRASHDIR/.config/git/attributes"
+	)
+'
+
 # For git var -l, we check only a representative variable;
 # testing the whole output would make our test too brittle with
 # respect to unrelated changes in the test suite's environment.

^ permalink raw reply related

* [PATCH v3 8/8] var: add config file locations
From: brian m. carlson @ 2023-06-27 16:19 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Junio C Hamano, Eric Sunshine, Derrick Stolee
In-Reply-To: <20230627161902.754472-1-sandals@crustytoothpaste.net>

From: "brian m. carlson" <bk2204@github.com>

Much like with attributes files, sometimes programs would like to know
the location of configuration files at the global or system levels.
However, it isn't always clear where these may live, especially for the
system file, which may have been hard-coded at compile time or computed
dynamically based on the runtime prefix.

Since other parties cannot intuitively know how Git was compiled and
where it looks for these files, help them by providing variables that
can be queried.  Because we have multiple paths for global config
values, print them in order from highest to lowest priority, and be sure
to split on newlines so that "git var -l" produces two entries for the
global value.

However, be careful not to split all values on newlines, since our
editor values could well contain such characters, and we don't want to
split them in such a case.

Note in the documentation that some values may contain multiple paths
and that callers should be prepared for that fact.  This helps people
write code that will continue to work in the event we allow multiple
items elsewhere in the future.

Signed-off-by: brian m. carlson <bk2204@github.com>
---
 Documentation/git-var.txt | 14 +++++++++
 builtin/var.c             | 57 +++++++++++++++++++++++++++++++++-
 t/t0007-git-var.sh        | 65 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 135 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-var.txt b/Documentation/git-var.txt
index dfbe5cb52b..c38fb3968b 100644
--- a/Documentation/git-var.txt
+++ b/Documentation/git-var.txt
@@ -80,6 +80,20 @@ GIT_ATTR_SYSTEM::
 GIT_ATTR_GLOBAL::
     The path to the global (per-user) linkgit:gitattributes[5] file.
 
+GIT_CONFIG_SYSTEM::
+    The path to the system configuration file, if one is enabled.
+
+GIT_CONFIG_GLOBAL::
+    The path to the global (per-user) configuration files, if any.
+
+Most path values contain only one value. However, some can contain multiple
+values, which are separated by newlines, and are listed in order from highest to
+lowest priority.  Callers should be prepared for any such path value to contain
+multiple items.
+
+Note that paths are printed even if they do not exist, but not if they are
+disabled by other environment variables.
+
 SEE ALSO
 --------
 linkgit:git-commit-tree[1]
diff --git a/builtin/var.c b/builtin/var.c
index 79f7bdf55f..ef45710a20 100644
--- a/builtin/var.c
+++ b/builtin/var.c
@@ -72,9 +72,45 @@ static char *git_attr_val_global(int ident_flag UNUSED)
 	return NULL;
 }
 
+static char *git_config_val_system(int ident_flag UNUSED)
+{
+	if (git_config_system()) {
+		char *file = git_system_config();
+		normalize_path_copy(file, file);
+		return file;
+	}
+	return NULL;
+}
+
+static char *git_config_val_global(int ident_flag UNUSED)
+{
+	struct strbuf buf = STRBUF_INIT;
+	char *user, *xdg;
+	size_t unused;
+
+	git_global_config(&user, &xdg);
+	if (xdg && *xdg) {
+		normalize_path_copy(xdg, xdg);
+		strbuf_addf(&buf, "%s\n", xdg);
+	}
+	if (user && *user) {
+		normalize_path_copy(user, user);
+		strbuf_addf(&buf, "%s\n", user);
+	}
+	free(xdg);
+	free(user);
+	strbuf_trim_trailing_newline(&buf);
+	if (buf.len == 0) {
+		strbuf_release(&buf);
+		return NULL;
+	}
+	return strbuf_detach(&buf, &unused);
+}
+
 struct git_var {
 	const char *name;
 	char *(*read)(int);
+	int multivalued;
 };
 static struct git_var git_vars[] = {
 	{
@@ -113,6 +149,15 @@ static struct git_var git_vars[] = {
 		.name = "GIT_ATTR_GLOBAL",
 		.read = git_attr_val_global,
 	},
+	{
+		.name = "GIT_CONFIG_SYSTEM",
+		.read = git_config_val_system,
+	},
+	{
+		.name = "GIT_CONFIG_GLOBAL",
+		.read = git_config_val_global,
+		.multivalued = 1,
+	},
 	{
 		.name = "",
 		.read = NULL,
@@ -126,7 +171,17 @@ static void list_vars(void)
 
 	for (ptr = git_vars; ptr->read; ptr++)
 		if ((val = ptr->read(0))) {
-			printf("%s=%s\n", ptr->name, val);
+			if (ptr->multivalued && *val) {
+				struct string_list list = STRING_LIST_INIT_DUP;
+				int i;
+
+				string_list_split(&list, val, '\n', -1);
+				for (i = 0; i < list.nr; i++)
+					printf("%s=%s\n", ptr->name, list.items[i].string);
+				string_list_clear(&list, 0);
+			} else {
+				printf("%s=%s\n", ptr->name, val);
+			}
 			free(val);
 		}
 }
diff --git a/t/t0007-git-var.sh b/t/t0007-git-var.sh
index 374d9f49e9..8cb597f99c 100755
--- a/t/t0007-git-var.sh
+++ b/t/t0007-git-var.sh
@@ -182,6 +182,49 @@ test_expect_success 'GIT_ATTR_GLOBAL points to the correct location' '
 	)
 '
 
+test_expect_success 'GIT_CONFIG_SYSTEM points to the correct location' '
+	TRASHDIR="$(test-tool path-utils normalize_path_copy "$(pwd)")" &&
+	test_must_fail env GIT_CONFIG_NOSYSTEM=1 git var GIT_CONFIG_SYSTEM &&
+	(
+		sane_unset GIT_CONFIG_NOSYSTEM &&
+		systempath=$(git var GIT_CONFIG_SYSTEM) &&
+		test "$systempath" != "" &&
+		systempath=$(GIT_CONFIG_SYSTEM=/dev/null git var GIT_CONFIG_SYSTEM) &&
+		if test_have_prereq MINGW
+		then
+			test "$systempath" = "nul"
+		else
+			test "$systempath" = "/dev/null"
+		fi &&
+		systempath=$(GIT_CONFIG_SYSTEM="$TRASHDIR/gitconfig" git var GIT_CONFIG_SYSTEM) &&
+		test "$systempath" = "$TRASHDIR/gitconfig"
+	)
+'
+
+test_expect_success 'GIT_CONFIG_GLOBAL points to the correct location' '
+	TRASHDIR="$(test-tool path-utils normalize_path_copy "$(pwd)")" &&
+	HOME="$TRASHDIR" XDG_CONFIG_HOME="$TRASHDIR/foo" git var GIT_CONFIG_GLOBAL >actual &&
+	echo "$TRASHDIR/foo/git/config" >expected &&
+	echo "$TRASHDIR/.gitconfig" >>expected &&
+	test_cmp expected actual &&
+	(
+		sane_unset XDG_CONFIG_HOME &&
+		HOME="$TRASHDIR" git var GIT_CONFIG_GLOBAL >actual &&
+		echo "$TRASHDIR/.config/git/config" >expected &&
+		echo "$TRASHDIR/.gitconfig" >>expected &&
+		test_cmp expected actual &&
+		globalpath=$(GIT_CONFIG_GLOBAL=/dev/null git var GIT_CONFIG_GLOBAL) &&
+		if test_have_prereq MINGW
+		then
+			test "$globalpath" = "nul"
+		else
+			test "$globalpath" = "/dev/null"
+		fi &&
+		globalpath=$(GIT_CONFIG_GLOBAL="$TRASHDIR/gitconfig" git var GIT_CONFIG_GLOBAL) &&
+		test "$globalpath" = "$TRASHDIR/gitconfig"
+	)
+'
+
 # For git var -l, we check only a representative variable;
 # testing the whole output would make our test too brittle with
 # respect to unrelated changes in the test suite's environment.
@@ -199,6 +242,28 @@ test_expect_success 'git var -l lists config' '
 	test_cmp expect actual.bare
 '
 
+test_expect_success 'git var -l lists multiple global configs' '
+	TRASHDIR="$(test-tool path-utils normalize_path_copy "$(pwd)")" &&
+	HOME="$TRASHDIR" XDG_CONFIG_HOME="$TRASHDIR/foo" git var -l >actual &&
+	grep "^GIT_CONFIG_GLOBAL=" actual >filtered &&
+	echo "GIT_CONFIG_GLOBAL=$TRASHDIR/foo/git/config" >expected &&
+	echo "GIT_CONFIG_GLOBAL=$TRASHDIR/.gitconfig" >>expected &&
+	test_cmp expected filtered
+'
+
+test_expect_success 'git var -l does not split multiline editors' '
+	(
+		GIT_EDITOR="!f() {
+			echo Hello!
+		}; f" &&
+		export GIT_EDITOR &&
+		echo "GIT_EDITOR=$GIT_EDITOR" >expected &&
+		git var -l >var &&
+		sed -n -e "/^GIT_EDITOR/,\$p" var | head -n 3 >actual &&
+		test_cmp expected actual
+	)
+'
+
 test_expect_success 'listing and asking for variables are exclusive' '
 	test_must_fail git var -l GIT_COMMITTER_IDENT
 '

^ permalink raw reply related

* [PATCH v3 5/8] var: adjust memory allocation for strings
From: brian m. carlson @ 2023-06-27 16:18 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Junio C Hamano, Eric Sunshine, Derrick Stolee
In-Reply-To: <20230627161902.754472-1-sandals@crustytoothpaste.net>

From: "brian m. carlson" <bk2204@github.com>

Right now, all of our values are constants whose allocation is managed
elsewhere.  However, in the future, we'll have some variables whose
memory we will need to free.  To keep things consistent, let's make each
of our functions allocate its own memory and make the caller responsible
for freeing it.

Signed-off-by: brian m. carlson <bk2204@github.com>
---
 builtin/var.c | 45 +++++++++++++++++++++++++++++----------------
 1 file changed, 29 insertions(+), 16 deletions(-)

diff --git a/builtin/var.c b/builtin/var.c
index 379564a399..d6f9f495c9 100644
--- a/builtin/var.c
+++ b/builtin/var.c
@@ -12,47 +12,57 @@
 
 static const char var_usage[] = "git var (-l | <variable>)";
 
-static const char *editor(int ident_flag UNUSED)
+static char *committer(int ident_flag)
 {
-	return git_editor();
+	return xstrdup_or_null(git_committer_info(ident_flag));
 }
 
-static const char *sequence_editor(int ident_flag UNUSED)
+static char *author(int ident_flag)
 {
-	return git_sequence_editor();
+	return xstrdup_or_null(git_author_info(ident_flag));
 }
 
-static const char *pager(int ident_flag UNUSED)
+static char *editor(int ident_flag UNUSED)
+{
+	return xstrdup_or_null(git_editor());
+}
+
+static char *sequence_editor(int ident_flag UNUSED)
+{
+	return xstrdup_or_null(git_sequence_editor());
+}
+
+static char *pager(int ident_flag UNUSED)
 {
 	const char *pgm = git_pager(1);
 
 	if (!pgm)
 		pgm = "cat";
-	return pgm;
+	return xstrdup(pgm);
 }
 
-static const char *default_branch(int ident_flag UNUSED)
+static char *default_branch(int ident_flag UNUSED)
 {
-	return git_default_branch_name(1);
+	return xstrdup_or_null(git_default_branch_name(1));
 }
 
-static const char *shell_path(int ident_flag UNUSED)
+static char *shell_path(int ident_flag UNUSED)
 {
-	return SHELL_PATH;
+	return xstrdup(SHELL_PATH);
 }
 
 struct git_var {
 	const char *name;
-	const char *(*read)(int);
+	char *(*read)(int);
 };
 static struct git_var git_vars[] = {
 	{
 		.name = "GIT_COMMITTER_IDENT",
-		.read = git_committer_info,
+		.read = committer,
 	},
 	{
 		.name = "GIT_AUTHOR_IDENT",
-		.read = git_author_info,
+		.read = author,
 	},
 	{
 		.name = "GIT_EDITOR",
@@ -83,11 +93,13 @@ static struct git_var git_vars[] = {
 static void list_vars(void)
 {
 	struct git_var *ptr;
-	const char *val;
+	char *val;
 
 	for (ptr = git_vars; ptr->read; ptr++)
-		if ((val = ptr->read(0)))
+		if ((val = ptr->read(0))) {
 			printf("%s=%s\n", ptr->name, val);
+			free(val);
+		}
 }
 
 static const struct git_var *get_git_var(const char *var)
@@ -113,7 +125,7 @@ static int show_config(const char *var, const char *value, void *cb)
 int cmd_var(int argc, const char **argv, const char *prefix UNUSED)
 {
 	const struct git_var *git_var;
-	const char *val;
+	char *val;
 
 	if (argc != 2)
 		usage(var_usage);
@@ -134,6 +146,7 @@ int cmd_var(int argc, const char **argv, const char *prefix UNUSED)
 		return 1;
 
 	printf("%s\n", val);
+	free(val);
 
 	return 0;
 }

^ permalink raw reply related

* [PATCH v3 0/8] Additional variables for git var
From: brian m. carlson @ 2023-06-27 16:18 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Junio C Hamano, Eric Sunshine, Derrick Stolee
In-Reply-To: <20230622195059.320593-1-sandals@crustytoothpaste.net>

On many Unix systems, we have a good idea where Git's configuration
files and the shell it uses are located.  However, there are some
systems where that's not the case, such as Windows and with Homebrew,
where the expected files might be found in another location.

Right now, programs who would like to access things like the system
gitattributes or config file have to guess where the distributor of Git
placed these files, and with runtime relocation, it's not even
guaranteed that these will be in a fixed place from invocation to
invocation.  As a result, we need a way to query Git about the location
of these files.

This series introduces five new configuration variables that refer to
the shell path, the system and global gitattributes files, and the
system and global config files.  The global values are not technically
needed, since they should be computable from the environment alone, but
they are added to make life easier for users.

My intention is that this is the last round of this series, but if folks
have other comments, please let me know.

Changes since v2:
* Pick up Peff's patch.
* Rename function arguments.
* Mark unused function arguments as unused.

Changes since v1:
* Format variables with C99 initializers.
* Minimize use of temporary files in the tests.
* Remove debugging statement.
* Avoid grep where possible in tests.
* Duplicate memory rather than optionally choosing whether to free data.
* Add and document test_file_is_executable.
* Possibly more things which I have forgotten.

Jeff King (1):
  var: mark unused parameters in git_var callbacks

brian m. carlson (7):
  t: add a function to check executable bit
  var: add support for listing the shell
  var: format variable structure with C99 initializers
  var: adjust memory allocation for strings
  attr: expose and rename accessor functions
  var: add attributes files locations
  var: add config file locations

 Documentation/git-var.txt |  23 ++++++
 attr.c                    |  14 ++--
 attr.h                    |   9 ++
 builtin/var.c             | 167 +++++++++++++++++++++++++++++++++-----
 t/README                  |   6 ++
 t/t0007-git-var.sh        | 100 +++++++++++++++++++++++
 t/test-lib-functions.sh   |   9 ++
 7 files changed, 301 insertions(+), 27 deletions(-)

Range-diff against v2:
-:  ---------- > 1:  5ed9f04f5b var: mark unused parameters in git_var callbacks
1:  20b7b85e98 = 2:  e7b676b6ae t: add a function to check executable bit
2:  7d92b4155f ! 3:  eae2f049cc var: add support for listing the shell
    @@ Documentation/git-var.txt: endif::git-default-pager[]
      linkgit:git-commit-tree[1]
     
      ## builtin/var.c ##
    -@@ builtin/var.c: static const char *default_branch(int flag)
    +@@ builtin/var.c: static const char *default_branch(int ident_flag UNUSED)
      	return git_default_branch_name(1);
      }
      
    -+static const char *shell_path(int flag)
    ++static const char *shell_path(int ident_flag UNUSED)
     +{
     +	return SHELL_PATH;
     +}
3:  29c338d59c = 4:  884f7a2967 var: format variable structure with C99 initializers
4:  2a2a762c44 ! 5:  c609b0a05c var: adjust memory allocation for strings
    @@ builtin/var.c
      
      static const char var_usage[] = "git var (-l | <variable>)";
      
    --static const char *editor(int flag)
    -+static char *committer(int flag)
    +-static const char *editor(int ident_flag UNUSED)
    ++static char *committer(int ident_flag)
      {
     -	return git_editor();
    -+	return xstrdup_or_null(git_committer_info(flag));
    ++	return xstrdup_or_null(git_committer_info(ident_flag));
      }
      
    --static const char *sequence_editor(int flag)
    -+static char *author(int flag)
    +-static const char *sequence_editor(int ident_flag UNUSED)
    ++static char *author(int ident_flag)
      {
     -	return git_sequence_editor();
    -+	return xstrdup_or_null(git_author_info(flag));
    ++	return xstrdup_or_null(git_author_info(ident_flag));
      }
      
    --static const char *pager(int flag)
    -+static char *editor(int flag)
    +-static const char *pager(int ident_flag UNUSED)
    ++static char *editor(int ident_flag UNUSED)
     +{
     +	return xstrdup_or_null(git_editor());
     +}
     +
    -+static char *sequence_editor(int flag)
    ++static char *sequence_editor(int ident_flag UNUSED)
     +{
     +	return xstrdup_or_null(git_sequence_editor());
     +}
     +
    -+static char *pager(int flag)
    ++static char *pager(int ident_flag UNUSED)
      {
      	const char *pgm = git_pager(1);
      
    @@ builtin/var.c
     +	return xstrdup(pgm);
      }
      
    --static const char *default_branch(int flag)
    -+static char *default_branch(int flag)
    +-static const char *default_branch(int ident_flag UNUSED)
    ++static char *default_branch(int ident_flag UNUSED)
      {
     -	return git_default_branch_name(1);
     +	return xstrdup_or_null(git_default_branch_name(1));
      }
      
    --static const char *shell_path(int flag)
    -+static char *shell_path(int flag)
    +-static const char *shell_path(int ident_flag UNUSED)
    ++static char *shell_path(int ident_flag UNUSED)
      {
     -	return SHELL_PATH;
     +	return xstrdup(SHELL_PATH);
5:  c0c5c59df9 = 6:  d029af8675 attr: expose and rename accessor functions
6:  49a04bd142 ! 7:  334df489f0 var: add attributes files locations
    @@ builtin/var.c
      #include "config.h"
      #include "editor.h"
      #include "ident.h"
    -@@ builtin/var.c: static char *shell_path(int flag)
    +@@ builtin/var.c: static char *shell_path(int ident_flag UNUSED)
      	return xstrdup(SHELL_PATH);
      }
      
    -+static char *git_attr_val_system(int flag)
    ++static char *git_attr_val_system(int ident_flag UNUSED)
     +{
     +	if (git_attr_system_is_enabled()) {
     +		char *file = xstrdup(git_attr_system_file());
    @@ builtin/var.c: static char *shell_path(int flag)
     +	return NULL;
     +}
     +
    -+static char *git_attr_val_global(int flag)
    ++static char *git_attr_val_global(int ident_flag UNUSED)
     +{
     +	char *file = xstrdup(git_attr_global_file());
     +	if (file) {
7:  a8b4d9396b ! 8:  abf079a923 var: add config file locations
    @@ Documentation/git-var.txt: GIT_ATTR_SYSTEM::
      linkgit:git-commit-tree[1]
     
      ## builtin/var.c ##
    -@@ builtin/var.c: static char *git_attr_val_global(int flag)
    +@@ builtin/var.c: static char *git_attr_val_global(int ident_flag UNUSED)
      	return NULL;
      }
      
    -+static char *git_config_val_system(int flag)
    ++static char *git_config_val_system(int ident_flag UNUSED)
     +{
     +	if (git_config_system()) {
     +		char *file = git_system_config();
    @@ builtin/var.c: static char *git_attr_val_global(int flag)
     +	return NULL;
     +}
     +
    -+static char *git_config_val_global(int flag)
    ++static char *git_config_val_global(int ident_flag UNUSED)
     +{
     +	struct strbuf buf = STRBUF_INIT;
     +	char *user, *xdg;
    @@ builtin/var.c: static char *git_attr_val_global(int flag)
      };
      static struct git_var git_vars[] = {
      	{
    - 		.name = "GIT_COMMITTER_IDENT",
    - 		.read = committer,
    -+		.multivalued = 0,
    - 	},
    - 	{
    - 		.name = "GIT_AUTHOR_IDENT",
    - 		.read = author,
    -+		.multivalued = 0,
    - 	},
    - 	{
    - 		.name = "GIT_EDITOR",
    - 		.read = editor,
    -+		.multivalued = 0,
    - 	},
    - 	{
    - 		.name = "GIT_SEQUENCE_EDITOR",
    - 		.read = sequence_editor,
    -+		.multivalued = 0,
    - 	},
    - 	{
    - 		.name = "GIT_PAGER",
    - 		.read = pager,
    -+		.multivalued = 0,
    - 	},
    - 	{
    - 		.name = "GIT_DEFAULT_BRANCH",
    - 		.read = default_branch,
    -+		.multivalued = 0,
    - 	},
    - 	{
    - 		.name = "GIT_SHELL_PATH",
    - 		.read = shell_path,
    -+		.multivalued = 0,
    - 	},
    - 	{
    - 		.name = "GIT_ATTR_SYSTEM",
    - 		.read = git_attr_val_system,
    -+		.multivalued = 0,
    - 	},
    - 	{
    +@@ builtin/var.c: static struct git_var git_vars[] = {
      		.name = "GIT_ATTR_GLOBAL",
      		.read = git_attr_val_global,
    -+		.multivalued = 0,
    -+	},
    + 	},
     +	{
     +		.name = "GIT_CONFIG_SYSTEM",
     +		.read = git_config_val_system,
    -+		.multivalued = 0,
     +	},
     +	{
     +		.name = "GIT_CONFIG_GLOBAL",
     +		.read = git_config_val_global,
     +		.multivalued = 1,
    - 	},
    ++	},
      	{
      		.name = "",
      		.read = NULL,
    -+		.multivalued = 0,
    - 	},
    - };
    - 
     @@ builtin/var.c: static void list_vars(void)
      
      	for (ptr = git_vars; ptr->read; ptr++)

^ permalink raw reply

* [PATCH v3 3/8] var: add support for listing the shell
From: brian m. carlson @ 2023-06-27 16:18 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Junio C Hamano, Eric Sunshine, Derrick Stolee
In-Reply-To: <20230627161902.754472-1-sandals@crustytoothpaste.net>

From: "brian m. carlson" <bk2204@github.com>

On most Unix systems, finding a suitable shell is easy: one simply uses
"sh" with an appropriate PATH value.  However, in many Windows
environments, the shell is shipped alongside Git, and it may or may not
be in PATH, even if Git is.

In such an environment, it can be very helpful to query Git for the
shell it's using, since other tools may want to use the same shell as
well.  To help them out, let's add a variable, GIT_SHELL_PATH, that
points to the location of the shell.

On Unix, we know our shell must be executable to be functional, so
assume that the distributor has correctly configured their environment,
and use that as a basic test.  On Git for Windows, we know that our
shell will be one of a few fixed values, all of which end in "sh" (such
as "bash").  This seems like it might be a nice test on Unix as well,
since it is customary for all shells to end in "sh", but there probably
exist such systems that don't have such a configuration, so be careful
here not to break them.

Signed-off-by: brian m. carlson <bk2204@github.com>
---
 Documentation/git-var.txt |  3 +++
 builtin/var.c             |  6 ++++++
 t/t0007-git-var.sh        | 15 +++++++++++++++
 3 files changed, 24 insertions(+)

diff --git a/Documentation/git-var.txt b/Documentation/git-var.txt
index f40202b8e3..f0f647e14a 100644
--- a/Documentation/git-var.txt
+++ b/Documentation/git-var.txt
@@ -71,6 +71,9 @@ endif::git-default-pager[]
 GIT_DEFAULT_BRANCH::
     The name of the first branch created in newly initialized repositories.
 
+GIT_SHELL_PATH::
+    The path of the binary providing the POSIX shell for commands which use the shell.
+
 SEE ALSO
 --------
 linkgit:git-commit-tree[1]
diff --git a/builtin/var.c b/builtin/var.c
index 10ee62f84c..bd340c5717 100644
--- a/builtin/var.c
+++ b/builtin/var.c
@@ -36,6 +36,11 @@ static const char *default_branch(int ident_flag UNUSED)
 	return git_default_branch_name(1);
 }
 
+static const char *shell_path(int ident_flag UNUSED)
+{
+	return SHELL_PATH;
+}
+
 struct git_var {
 	const char *name;
 	const char *(*read)(int);
@@ -47,6 +52,7 @@ static struct git_var git_vars[] = {
 	{ "GIT_SEQUENCE_EDITOR", sequence_editor },
 	{ "GIT_PAGER", pager },
 	{ "GIT_DEFAULT_BRANCH", default_branch },
+	{ "GIT_SHELL_PATH", shell_path },
 	{ "", NULL },
 };
 
diff --git a/t/t0007-git-var.sh b/t/t0007-git-var.sh
index eeb8539c1b..e35f07afcb 100755
--- a/t/t0007-git-var.sh
+++ b/t/t0007-git-var.sh
@@ -147,6 +147,21 @@ test_expect_success 'get GIT_SEQUENCE_EDITOR with configuration and environment
 	)
 '
 
+test_expect_success POSIXPERM 'GIT_SHELL_PATH points to a valid executable' '
+	shellpath=$(git var GIT_SHELL_PATH) &&
+	test_path_is_executable "$shellpath"
+'
+
+# We know in this environment that our shell will be one of a few fixed values
+# that all end in "sh".
+test_expect_success MINGW 'GIT_SHELL_PATH points to a suitable shell' '
+	shellpath=$(git var GIT_SHELL_PATH) &&
+	case "$shellpath" in
+	*sh) ;;
+	*) return 1;;
+	esac
+'
+
 # For git var -l, we check only a representative variable;
 # testing the whole output would make our test too brittle with
 # respect to unrelated changes in the test suite's environment.

^ permalink raw reply related

* [PATCH v3 4/8] var: format variable structure with C99 initializers
From: brian m. carlson @ 2023-06-27 16:18 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Junio C Hamano, Eric Sunshine, Derrick Stolee
In-Reply-To: <20230627161902.754472-1-sandals@crustytoothpaste.net>

From: "brian m. carlson" <bk2204@github.com>

Right now, we have only two items in our variable struct.  However, in
the future, we're going to add two more items.  To help keep our diffs
nice and tidy and make this structure easier to read, switch to use
C99-style initializers for our data.

Signed-off-by: brian m. carlson <bk2204@github.com>
---
 builtin/var.c | 40 ++++++++++++++++++++++++++++++++--------
 1 file changed, 32 insertions(+), 8 deletions(-)

diff --git a/builtin/var.c b/builtin/var.c
index bd340c5717..379564a399 100644
--- a/builtin/var.c
+++ b/builtin/var.c
@@ -46,14 +46,38 @@ struct git_var {
 	const char *(*read)(int);
 };
 static struct git_var git_vars[] = {
-	{ "GIT_COMMITTER_IDENT", git_committer_info },
-	{ "GIT_AUTHOR_IDENT",   git_author_info },
-	{ "GIT_EDITOR", editor },
-	{ "GIT_SEQUENCE_EDITOR", sequence_editor },
-	{ "GIT_PAGER", pager },
-	{ "GIT_DEFAULT_BRANCH", default_branch },
-	{ "GIT_SHELL_PATH", shell_path },
-	{ "", NULL },
+	{
+		.name = "GIT_COMMITTER_IDENT",
+		.read = git_committer_info,
+	},
+	{
+		.name = "GIT_AUTHOR_IDENT",
+		.read = git_author_info,
+	},
+	{
+		.name = "GIT_EDITOR",
+		.read = editor,
+	},
+	{
+		.name = "GIT_SEQUENCE_EDITOR",
+		.read = sequence_editor,
+	},
+	{
+		.name = "GIT_PAGER",
+		.read = pager,
+	},
+	{
+		.name = "GIT_DEFAULT_BRANCH",
+		.read = default_branch,
+	},
+	{
+		.name = "GIT_SHELL_PATH",
+		.read = shell_path,
+	},
+	{
+		.name = "",
+		.read = NULL,
+	},
 };
 
 static void list_vars(void)

^ permalink raw reply related

* [PATCH v3 2/8] t: add a function to check executable bit
From: brian m. carlson @ 2023-06-27 16:18 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Junio C Hamano, Eric Sunshine, Derrick Stolee
In-Reply-To: <20230627161902.754472-1-sandals@crustytoothpaste.net>

From: "brian m. carlson" <bk2204@github.com>

In line with our other helper functions for paths, let's add a function
to check whether a path is executable, and if not, print a suitable
error message.  Document this function, and note that it must only be
used under the POSIXPERM prerequisite, since it doesn't otherwise work
on Windows.

Signed-off-by: brian m. carlson <bk2204@github.com>
---
 t/README                | 6 ++++++
 t/test-lib-functions.sh | 9 +++++++++
 2 files changed, 15 insertions(+)

diff --git a/t/README b/t/README
index bdfac4cceb..3e155011de 100644
--- a/t/README
+++ b/t/README
@@ -1098,6 +1098,12 @@ see test-lib-functions.sh for the full list and their options.
    the symbolic link in the file system and a part that does; then only
    the latter part need be protected by a SYMLINKS prerequisite (see below).
 
+ - test_path_is_executable
+
+   This tests whether a file is executable and prints an error message
+   if not. This must be used only under the POSIXPERM prerequisite
+   (see below).
+
  - test_oid_init
 
    This function loads facts and useful object IDs related to the hash
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index b3864e22e9..2fa716c567 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -910,6 +910,15 @@ test_path_is_symlink () {
 	fi
 }
 
+test_path_is_executable () {
+	test "$#" -ne 1 && BUG "1 param"
+	if ! test -x "$1"
+	then
+		echo "$1 is not executable"
+		false
+	fi
+}
+
 # Check if the directory exists and is empty as expected, barf otherwise.
 test_dir_is_empty () {
 	test "$#" -ne 1 && BUG "1 param"

^ permalink raw reply related

* [PATCH v3 1/8] var: mark unused parameters in git_var callbacks
From: brian m. carlson @ 2023-06-27 16:18 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Junio C Hamano, Eric Sunshine, Derrick Stolee
In-Reply-To: <20230627161902.754472-1-sandals@crustytoothpaste.net>

From: Jeff King <peff@peff.net>

We abstract the set of variables into a table, with a "read" callback to
provide the value of each. Each callback takes a "flag" argument, but
most callbacks don't make use of it.

This flag is a bit odd. It may be set to IDENT_STRICT, which make sense
for ident-based callbacks, but is just confusing for things like
GIT_EDITOR.

At first glance, it seems like this is just a hack to let us directly
stick the generic git_committer_info() and git_author_info() functions
into our table. And we'd be better off to wrap them with local functions
which pass IDENT_STRICT, and have our callbacks take no option at all.

But that doesn't quite work. We pass IDENT_STRICT when the caller asks
for a specific variable, but otherwise do not (so that "git var -l" does
not bail if the committer ident cannot be formed).

So we really do need to pass in the flag to each invocation, even if the
individual callback doesn't care about it. Let's mark the unused ones so
that -Wunused-parameter does not complain. And while we're here, let's
rename them so that it's clear that the flag values we get will be from
the IDENT_* set. That may prevent confusion for future readers of the
code.

Another option would be to define our own local "strict" flag for the
callbacks, and then have wrappers that translate that to IDENT_STRICT
where it matters. But that would be more boilerplate for little gain
(most functions would still ignore the "strict" flag anyway).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: brian m. carlson <bk2204@github.com>
---
 builtin/var.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/builtin/var.c b/builtin/var.c
index 2149998980..10ee62f84c 100644
--- a/builtin/var.c
+++ b/builtin/var.c
@@ -12,17 +12,17 @@
 
 static const char var_usage[] = "git var (-l | <variable>)";
 
-static const char *editor(int flag)
+static const char *editor(int ident_flag UNUSED)
 {
 	return git_editor();
 }
 
-static const char *sequence_editor(int flag)
+static const char *sequence_editor(int ident_flag UNUSED)
 {
 	return git_sequence_editor();
 }
 
-static const char *pager(int flag)
+static const char *pager(int ident_flag UNUSED)
 {
 	const char *pgm = git_pager(1);
 
@@ -31,7 +31,7 @@ static const char *pager(int flag)
 	return pgm;
 }
 
-static const char *default_branch(int flag)
+static const char *default_branch(int ident_flag UNUSED)
 {
 	return git_default_branch_name(1);
 }

^ permalink raw reply related

* Re: [PATCH v2 6/7] var: add attributes files locations
From: brian m. carlson @ 2023-06-27 16:12 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Junio C Hamano, Eric Sunshine, Derrick Stolee
In-Reply-To: <20230627070557.GB1226768@coredump.intra.peff.net>

[-- Attachment #1: Type: text/plain, Size: 1822 bytes --]

On 2023-06-27 at 07:05:57, Jeff King wrote:
> On Mon, Jun 26, 2023 at 07:00:07PM +0000, brian m. carlson wrote:
> 
> > @@ -51,6 +52,26 @@ static char *shell_path(int flag)
> >  	return xstrdup(SHELL_PATH);
> >  }
> >  
> > +static char *git_attr_val_system(int flag)
> > +{
> > +	if (git_attr_system_is_enabled()) {
> > +		char *file = xstrdup(git_attr_system_file());
> > +		normalize_path_copy(file, file);
> > +		return file;
> > +	}
> > +	return NULL;
> > +}
> 
> These new ones would ideally mark the "flag" variable with the UNUSED
> attribute (in preparation for building with -Wunused-parameter).
> 
> I can also come through later and fix them up in a separate patch. It's
> slightly awkward, just because I was about to post a patch that fixed
> the existing functions in that file, and I'd have to either rebase on
> top, or make a second pass once this is merged.
> 
> That said, I also renamed the "flag" variable in my patch because it's
> super confusing (see my patch below for reference). So adjusting your
> new callers to match (without my changes) would be a little weird. The
> least-weird thing would be sticking my patch at the front of your
> series, but I don't want to make you do extra work.
> 
> So I dunno. I'm mostly giving a heads-up, and seeing if you (or other
> reviewers in the thread) have an idea to make this "flag" thing less
> awful. I'm also happy to just do my topic separately, and then
> eventually circle back after yours is merged.

I've picked up your patch as the first patch in the series and will send
it out in v3 in just a few minutes.  Since I plan to have v3 be the last
round of this series, I'll let you send out any further changes as
fixups on top of that.
-- 
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

^ permalink raw reply

* Bug Report
From: Tiago d'Almeida @ 2023-06-27 16:02 UTC (permalink / raw)
  To: git

[-- Attachment #1: Type: text/plain, Size: 991 bytes --]

Hello!

I'm Tiago and sorry for any mistake because it is my first bug report to git.

So after git updated, I begin to get some errors and unexpected git
status output.

I made a test git repo to test this, and here are the results:

```
mkdir git_bug && cd git_bug
git init -q && git maintenance register && echo "Done!"
touch <files>
mkdir <folders>
touch <files in folders>
git status --short

## No commits yet on main
?? LICENSE
?? README.md
?? folder/
?? folder_1/

git add <files>
git status

## No commits yet on main
?? LICENSE
?? README.md
?? folder/
?? folder_1/

git commit -s -m "First commit"

On branch main

Initial commit

Untracked files:
LICENSE
README.md
folder/
folder_1/

nothing added to commit but untracked files present

```

Attached to this email follow the `git bugreport` and global `config`
files, and the git_bug repo.

I've checked my global config file and it seems all ok, but if there
is some error or contradicition, please tell me.

Thanks in advance!

[-- Attachment #2: git-bugreport-2023-06-27-1643.txt --]
[-- Type: text/plain, Size: 971 bytes --]

Thank you for filling out a Git bug report!
Please answer the following questions to help us understand your issue.

What did you do before the bug happened? (Steps to reproduce your issue)

A normal git routine.
- git add <files>
- git commit -s -m "Commit"

What did you expect to happen? (Expected behavior)

No errors and the files being added and commited.

What happened instead? (Actual behavior)

No files added. Error when tried to commit.

What's different between what you expected and what actually happened?

The error.

Anything else you want to add:

This just happened after updating git to version 2.41 


[System Info]
git version 2.41.0
cpu: x86_64
no commit associated with this build
sizeof-long: 8
sizeof-size_t: 8
shell-path: /bin/sh
uname: Linux 6.3.8-200.fc38.x86_64 #1 SMP PREEMPT_DYNAMIC Thu Jun 15 02:15:40 UTC 2023 x86_64
compiler info: gnuc: 13.1
libc info: glibc: 2.37
$SHELL (typically, interactive shell): /usr/bin/fish


[Enabled Hooks]

[-- Attachment #3: global_config --]
[-- Type: application/octet-stream, Size: 7090 bytes --]

# TIAGO d'ALMEIDA GIT CONFIG


# These directives includes the user.name and user.email according to the repo location.
[include]
  path = "./config_d/personal"

[includeIf "gitdir:~/Projects/work/"]
  path = "./config_d/work"

[includeIf "gitdir:~/Work/"]
  path = "./config_d/work"

[alias]
  a   = add
  aa  = add --all .
  ai  = add -i
  aac = ! git add --all . && git commit -m
  br  = branch
  cl  = clone
  cm  = commit -s -m
  cml = commit -s
  cp  = cherry-pick
  d   = diff
  f   = fetch
  fa  = fetch --all
  i   = ! git init -q && git maintenance register && echo "Done!"
  l   = log --graph --oneline -n 10
  ls  = log --graph --oneline --all
  lch = log --pretty="format:(%h) %N %an" --show-notes=*
  m   = maintenance run
  ms  = maintenance register
  mu  = maintenance unregister
  nt  = notes
  nta = notes append
  nch = notes append --ref=changelog
  p   = push
  pa  = push --all
  rb  = rebase -i
  r   = reset
  r1  = reset HEAD~
  r2  = reset HEAD~~
  rh  = reset --hard
  rh1 = reset HEAD~ --hard
  rh2 = reset HEAD~~ --hard
  rt  = restore
  s   = status --short
  sl  = status --long
  st  = stash              # equivalent to `stash push`
  stp = stash pop
  stl = stash list
  sta = stash apply
  sts = stash show
  sw  = switch             # switch and restore is the new checkout
  w   = ! watch -n 5 -p -c git -c color.ui=always status

  amend   = commit -s --amend --no-edit
  fixup   = commit -s --fixup

  since   = log --graph --oneline --since="1 week ago"

  track   = add -N
  unstage = restore --staged
  forget  = rm -r --cached

  del     = delete
  delete  = ! git maintenance unregister && rm -rf .git && echo "Done!"
  watch   = ! watch -n 5 -p -c git -c color.ui=always status

  show    = show --show-signature

#	brd = branch -d
#	brD = branch -D
# br_desc = branch --edit-description
#	merged = branch --merged
#	dev = !git checkout dev && git pull origin dev
#	staging = !git checkout staging && git pull origin staging
#	master = !git checkout master && git pull origin 
#	po = push origin
#	pu = !git push origin `git branch --show-current`
#	pod = push origin dev
#	pos = push origin staging
#	pom = push origin main
#	poh = push origin HEAD
#	pogm = !git push origin gh-pages && git checkout master && git pull origin master && git rebase gh-pages && git push origin master && git checkout gh-pages
#	pomg = !git push origin master && git checkout gh-pages && git pull origin gh-pages && git rebase master && git push origin gh-pages && git checkout master
#	plo = pull origin
#	plod = pull origin dev
#	plos = pull origin staging
#	plom = pull origin master
#	ploh = pull origin HEAD
#	f = "!git ls-files | grep -i"
# gr = grep -Ii

[advice]
  addIgnoredFile = false
  statusHints = false

[branch]
  autoSetupMerge = always

[core]
	askPass = ""
  compression = 6
  editor = nano +1
  eol = lf
  filemode = true
  fsmonitor = true
  fsmonitorHookVersion = 2
  fsyncMethod = batch
#  hooksPath = "./hooks" --> can't be this path, correct this in the future; see man git-config
#  notes = refs/notes/commits
  pager = less --tabs=2
#  quotePath = off
  safecrlf = true
  sparseCheckout = true
  sparseCheckoutCone = true
  splitIndex = true
  symlinks = true
  whitespace = space-before-tab, tab-in-indent, trailing-space, tabwidth=2

[add]
#  ignoreErrors = true

[apply]
  ignoreWhitespace = false

[checkout]
  defaultRemote = origin
  workers = 0
  thresholdForParallelism = 100
  
[color]
  ui = auto

[color "advice"]
  hint = dim white

[color "status"]
  header       = italic white
  added        = green
  updated      = cyan
  changed      = brightyellow
  untracked    = dim white
  branch       = bold magenta
  nobranch     = red
  localBranch  = bold magenta
  remoteBranch = brightgreen
  unmerged     = cyan

[column]
  ui = auto, column, dense

[commit]
  gpgSign = true
#  template = "./commit"

[credential]
#  helper = "cache --timeout 18000 --socket ~/.config/git/socket"
  helper = "cache --timeout 18000"

[diff]
  context = 5
  mnemonicPrefix = true
  relative = true
  renames = copies
  algoritm = patience
  wsErrorHighlight = all
  colorMoved = true

[feature]
  experimental = true
  manyFiles = true

[fetch]
  prune = true
  pruneTags = true
  output = compact
  parallel = 3
  writeCommitGraph = true

[filter "lfs"]
  clean = git-lfs clean -- %f
  smudge = git-lfs smudge -- %f
  process = git-lfs filter-process
  required = true

[gc]
  bigPackThreshold    = 1g
  logExpiry           = 1.week
  cruftPacks          = true
  pruneExpire         = 1.week
  worktreePruneExpire = 1.month

[gpg]
  program = gpg2
  minTrustLevel = fully

[help]
  autoCorrect = prompt

[http]
#  cookieFile = ~/.config/git/cookie
  saveCookies = true
  version = HTTP/2
  sslVersion = tlsv1.3
  sslVerify = true

[i18n]
  commitEncoding = utf-8
  logOutputEncoding = utf-8

[index]
  sparse = true
  threads = true

[init]
  defaultBranch = main
#  templateDir = ~/.config/git/template/

[interactive]
  singleKey = true

[lfs "https://github.com/"]
  locksverify = true

[lfs "https://gitlab.com/"]
  locksverify = true

[log]
  decorate = auto
  graphColors = brightwhite, red, green, blue, magenta, yellow, cyan

[lsrefs]
  unborn = advertise

[maintenance]
  auto = false
  strategy = incremental
# commit-graph        - hourly
# prefetch            - hourly
# gc                  - off
# loose-objects       - daily
# incremental-repack  - daily
# pack-refs           - weekly

[maintenance.gc]
  enabled = true
  schedule= weekly

[merge]
  conflictStyle = merge
  ff = only
  verifySignatures = true
  branchdesc = true
  log = true
  renormalize = true
  stat = true
  autoStash = true

#[notes]
#  # https://dev.to/leehambley/effortlessly-maintain-a-high-quality-change-log-with-git-notes-4bm5
##  mergeStrategy = cat_sort_uniq
##  rewriteMode = cat_sort_uniq
##  rewriteRef = refs/notes/commits
#
#[notes "rewrite"]
#  amend = true
#  rebase = true

[pack]
  useBitmaps = true
  useSparse = true
  writeBitmapHashCache =true
  writeReverseIndex = true

# pass command-line tool 
[pass]
  signcommits = true

[protocol]
  version = 2

[pull]
  ff      = only
  rebase  = false
  twohead = ort

[push]
  autoSetupRemote = true
  default = current
  followTags = true
  gpgSign = if-asked
  negotiate = true

[rebase]
  backend = merge
  autoSquash = true
  autoStash = true
  missingCommitsCheck = warn
  abbreviateCommands = true

[remote]
  pushDefault = origin

#[remote "origin"]
#  tagOpt = "--tags"
##  fetch = +refs/heads/*:refs/remotes/origin/*
##  fetch = +refs/notes/*:refs/notes/*
##  push = +refs/heads/*
##  push = +refs/notes/*:refs/notes/*
#
[repack]
  writeBitmaps = true

[rerere]
  autoUpdate = true
  enabled = true

[safe]
  bareRepository = explicit

[splitIndex]
  maxPercentChange = 20

[ssh]
  variant = ssh

[status]
  branch    = true
  showStash = true

[transfer]
  credentialsInUrl = die
  fsckObjects = true

[tag]
  forceSignAnnotated = true
  gpgSign = true

[user]
  useConfigOnly = true
  signingkey = ***********

[versionsort]
  suffix = "-pre"
  suffix = "-rc"
  suffix = ""

[-- Attachment #4: git_bug.tar.gz --]
[-- Type: application/gzip, Size: 11601 bytes --]

^ permalink raw reply

* [PATCH 9/9] gitk: default select reset hard in dialog
From: Jens Lidestrom via GitGitGadget @ 2023-06-27 14:41 UTC (permalink / raw)
  To: git; +Cc: Paul Mackerras [ ], Jens Lidestrom, Jens Lidestrom
In-Reply-To: <pull.1551.git.1687876884.gitgitgadget@gmail.com>

From: Jens Lidestrom <jens@lidestrom.se>

Reset hard is dangerous but also the most common reset type, and not
having it pre-selected in the dialog is annoying to users.

It is also less dangerous in the GUI where there is a confirmation
dialog. Also, dangling commits remain in the GUI and can be recovered.

Signed-off-by: Jens Lidestrom <jens@lidestrom.se>
---
 gitk-git/gitk | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/gitk-git/gitk b/gitk-git/gitk
index 9d93053e360..5b0a0ea46be 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -9906,7 +9906,9 @@ proc resethead {reset_target_id} {
         [mc "Reset branch %s to %s?" $mainhead [commit_name $reset_target_id 1]]
     pack $w.m -side top -fill x -padx 20 -pady 20
     ${NS}::labelframe $w.f -text [mc "Reset type:"]
-    set resettype mixed
+    # Reset hard is dangerous but also the most common reset type, and not
+    # having it pre-selected in the dialog is annoying to users.
+    set resettype hard
     ${NS}::radiobutton $w.f.soft -value soft -variable resettype \
         -text [mc "Soft: Leave working tree and index untouched"]
     grid $w.f.soft -sticky w
-- 
gitgitgadget

^ permalink raw reply related

* [PATCH 8/9] gitk: focus ok button in reset dialog
From: Jens Lidestrom via GitGitGadget @ 2023-06-27 14:41 UTC (permalink / raw)
  To: git; +Cc: Paul Mackerras [ ], Jens Lidestrom, Jens Lidestrom
In-Reply-To: <pull.1551.git.1687876884.gitgitgadget@gmail.com>

From: Jens Lidestrom <jens@lidestrom.se>

This is more convenient for users, especially when using keyboard
commands.

Signed-off-by: Jens Lidestrom <jens@lidestrom.se>
---
 gitk-git/gitk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gitk-git/gitk b/gitk-git/gitk
index 5b01d1902a5..9d93053e360 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -9922,7 +9922,7 @@ proc resethead {reset_target_id} {
     ${NS}::button $w.cancel -text [mc Cancel] -command "destroy $w"
     bind $w <Key-Escape> [list destroy $w]
     pack $w.cancel -side right -fill x -padx 20 -pady 20
-    bind $w <Visibility> "grab $w; focus $w"
+    bind $w <Visibility> "grab $w; focus $w.ok"
     tkwait window $w
     if {!$confirm_ok} return
     if {[catch {set fd [open \
-- 
gitgitgadget


^ permalink raw reply related

* [PATCH 7/9] gitk: add keyboard bind to cherry-pick
From: Jens Lidestrom via GitGitGadget @ 2023-06-27 14:41 UTC (permalink / raw)
  To: git; +Cc: Paul Mackerras [ ], Jens Lidestrom, Jens Lidestrom
In-Reply-To: <pull.1551.git.1687876884.gitgitgadget@gmail.com>

From: Jens Lidestrom <jens@lidestrom.se>

Signed-off-by: Jens Lidestrom <jens@lidestrom.se>
---
 gitk-git/gitk | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/gitk-git/gitk b/gitk-git/gitk
index 0d83a72a424..5b01d1902a5 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -2690,6 +2690,7 @@ proc makewindow {} {
     bind $ctext $ctxbut {pop_diff_menu %W %X %Y %x %y}
     bind $ctext <Button-1> {focus %W}
     bind $ctext <<Selection>> rehighlight_search_results
+    bind . <$M1B-p> {cherrypick [selected_line_id]}
     bind . <$M1B-t> {resethead [selected_line_id]}
     bind . <$M1B-o> {checkout [selected_line_head] [selected_line_id]}
     bind . <$M1B-m> {rmbranch [selected_line_head] [selected_line_id] 1}
@@ -2711,7 +2712,7 @@ proc makewindow {} {
         {mc "Copy commit reference" command copyreference}
         {mc "Write commit to file" command writecommit}
         {mc "Create new branch" command {mkbranch $rowmenuid}}
-        {mc "Cherry-pick this commit" command cherrypick}
+        {mc "Cherry-pick this commit" command {cherrypick $rowmenuid}}
         {mc "Reset current branch to here" command {resethead $rowmenuid}}
         {mc "Mark this commit" command markhere}
         {mc "Return to mark" command gotomark}
@@ -3186,6 +3187,7 @@ proc keys {} {
 [mc "<%s-minus>	Decrease font size" $M1T]
 [mc "<F5>		Update"]
 [mc "<%s-T>		Reset current branch to selected commit" $M1T]
+[mc "<%s-P>		Cherry-pick selected commit to current branch" $M1T]
 [mc "<%s-O>		Check out selected commit" $M1T]
 [mc "<%s-C>		Create branch on selected commit" $M1T]
 [mc "<%s-M>		Remove selected branch" $M1T]
@@ -9758,24 +9760,29 @@ proc exec_citool {tool_args {baseid {}}} {
     array set env $save_env
 }
 
-proc cherrypick {} {
-    global rowmenuid curview
+proc cherrypick {id} {
+    global curview headids
     global mainhead mainheadid
     global gitdir
 
+    if {! [info exists headids($mainhead)]} {
+        error_popup [mc "Cannot cherry-pick to a detached head"]
+        return
+    }
+
     set oldhead [exec git rev-parse HEAD]
-    set dheads [descheads $rowmenuid]
+    set dheads [descheads $id]
     if {$dheads ne {} && [lsearch -exact $dheads $oldhead] >= 0} {
         set ok [confirm_popup [mc "Commit %s is already\
                 included in branch %s -- really re-apply it?" \
-                                   [string range $rowmenuid 0 7] $mainhead]]
+                                   [string range $id 0 7] $mainhead]]
         if {!$ok} return
     }
     nowbusy cherrypick [mc "Cherry-picking"]
     update
     # Unfortunately git-cherry-pick writes stuff to stderr even when
     # no error occurs, and exec takes that as an indication of error...
-    if {[catch {exec sh -c "git cherry-pick -r $rowmenuid 2>&1"} err]} {
+    if {[catch {exec sh -c "git cherry-pick -r $id 2>&1"} err]} {
         notbusy cherrypick
         if {[regexp -line \
                  {Entry '(.*)' (would be overwritten by merge|not uptodate)} \
@@ -9791,7 +9798,7 @@ proc cherrypick {} {
                         resolve it?"]]} {
                 # Force citool to read MERGE_MSG
                 file delete [file join $gitdir "GITGUI_MSG"]
-                exec_citool {} $rowmenuid
+                exec_citool {} $id
             }
         } else {
             error_popup $err
-- 
gitgitgadget


^ permalink raw reply related

* [PATCH 6/9] gitk: add keyboard bind for create and remove branch
From: Jens Lidestrom via GitGitGadget @ 2023-06-27 14:41 UTC (permalink / raw)
  To: git; +Cc: Paul Mackerras [ ], Jens Lidestrom, Jens Lidestrom
In-Reply-To: <pull.1551.git.1687876884.gitgitgadget@gmail.com>

From: Jens Lidestrom <jens@lidestrom.se>

Signed-off-by: Jens Lidestrom <jens@lidestrom.se>
---
 gitk-git/gitk | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/gitk-git/gitk b/gitk-git/gitk
index 596977abe89..0d83a72a424 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -2692,6 +2692,8 @@ proc makewindow {} {
     bind $ctext <<Selection>> rehighlight_search_results
     bind . <$M1B-t> {resethead [selected_line_id]}
     bind . <$M1B-o> {checkout [selected_line_head] [selected_line_id]}
+    bind . <$M1B-m> {rmbranch [selected_line_head] [selected_line_id] 1}
+    bind . <$M1B-b> {mkbranch [selected_line_id]}
     for {set i 1} {$i < 10} {incr i} {
         bind . <$M1B-Key-$i> [list go_to_parent $i]
     }
@@ -2735,7 +2737,7 @@ proc makewindow {} {
     makemenu $headctxmenu {
         {mc "Check out this branch" command {checkout $headmenuhead $headmenuid}}
         {mc "Rename this branch" command mvbranch}
-        {mc "Remove this branch" command rmbranch}
+        {mc "Remove this branch" command {rmbranch $headmenuhead $headmenuid 0}}
         {mc "Copy branch name" command {clipboard clear; clipboard append $headmenuhead}}
     }
     $headctxmenu configure -tearoff 0
@@ -3185,6 +3187,8 @@ proc keys {} {
 [mc "<F5>		Update"]
 [mc "<%s-T>		Reset current branch to selected commit" $M1T]
 [mc "<%s-O>		Check out selected commit" $M1T]
+[mc "<%s-C>		Create branch on selected commit" $M1T]
+[mc "<%s-M>		Remove selected branch" $M1T]
 " \
             -justify left -bg $bgcolor -border 2 -relief groove
     pack $w.m -side top -fill both -padx 2 -pady 2
@@ -9576,13 +9580,13 @@ proc wrcomcan {} {
     unset wrcomtop
 }
 
-proc mkbranch {} {
-    global NS rowmenuid
+proc mkbranch {id} {
+    global NS
 
     set top .branchdialog
 
     set val(name) ""
-    set val(id) $rowmenuid
+    set val(id) $id
     set val(command) [list mkbrgo $top]
 
     set ui(title) [mc "Create branch"]
@@ -10054,13 +10058,14 @@ proc readcheckoutstat {fd newhead newheadref newheadid} {
     }
 }
 
-proc rmbranch {} {
-    global headmenuid headmenuhead mainhead
+proc rmbranch {head id shouldComfirm} {
+    global mainhead
     global idheads
-
-    set head $headmenuhead
-    set id $headmenuid
     # this check shouldn't be needed any more...
+    if {$head eq ""} {
+        error_popup [mc "Cannot delete a detached head"]
+        return
+    }
     if {$head eq $mainhead} {
         error_popup [mc "Cannot delete the currently checked-out branch"]
         return
@@ -10070,6 +10075,8 @@ proc rmbranch {} {
         # the stuff on this branch isn't on any other branch
         if {![confirm_popup [mc "The commits on branch %s aren't on any other\
                         branch.\nReally delete branch %s?" $head $head]]} return
+    } elseif {$shouldComfirm} {
+        if {![confirm_popup [mc "Really delete branch %s?" $head]]} return
     }
     nowbusy rmbranch
     update
-- 
gitgitgadget


^ permalink raw reply related

* [PATCH 5/9] gitk: add keyboard bind for checkout
From: Jens Lidestrom via GitGitGadget @ 2023-06-27 14:41 UTC (permalink / raw)
  To: git; +Cc: Paul Mackerras [ ], Jens Lidestrom, Jens Lidestrom
In-Reply-To: <pull.1551.git.1687876884.gitgitgadget@gmail.com>

From: Jens Lidestrom <jens@lidestrom.se>

This also introduces the ability to check out detatched heads. This
shouldn't result any problems, because gitk already works with
detatched heads if they are checked out using the terminal.

Signed-off-by: Jens Lidestrom <jens@lidestrom.se>
---
 gitk-git/gitk | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/gitk-git/gitk b/gitk-git/gitk
index bfe912983f4..596977abe89 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -2691,6 +2691,7 @@ proc makewindow {} {
     bind $ctext <Button-1> {focus %W}
     bind $ctext <<Selection>> rehighlight_search_results
     bind . <$M1B-t> {resethead [selected_line_id]}
+    bind . <$M1B-o> {checkout [selected_line_head] [selected_line_id]}
     for {set i 1} {$i < 10} {incr i} {
         bind . <$M1B-Key-$i> [list go_to_parent $i]
     }
@@ -2707,7 +2708,7 @@ proc makewindow {} {
         {mc "Create tag" command mktag}
         {mc "Copy commit reference" command copyreference}
         {mc "Write commit to file" command writecommit}
-        {mc "Create new branch" command mkbranch}
+        {mc "Create new branch" command {mkbranch $rowmenuid}}
         {mc "Cherry-pick this commit" command cherrypick}
         {mc "Reset current branch to here" command {resethead $rowmenuid}}
         {mc "Mark this commit" command markhere}
@@ -2732,7 +2733,7 @@ proc makewindow {} {
 
     set headctxmenu .headctxmenu
     makemenu $headctxmenu {
-        {mc "Check out this branch" command cobranch}
+        {mc "Check out this branch" command {checkout $headmenuhead $headmenuid}}
         {mc "Rename this branch" command mvbranch}
         {mc "Remove this branch" command rmbranch}
         {mc "Copy branch name" command {clipboard clear; clipboard append $headmenuhead}}
@@ -3183,6 +3184,7 @@ proc keys {} {
 [mc "<%s-minus>	Decrease font size" $M1T]
 [mc "<F5>		Update"]
 [mc "<%s-T>		Reset current branch to selected commit" $M1T]
+[mc "<%s-O>		Check out selected commit" $M1T]
 " \
             -justify left -bg $bgcolor -border 2 -relief groove
     pack $w.m -side top -fill both -padx 2 -pady 2
@@ -9978,25 +9980,26 @@ proc headmenu {x y id head} {
     tk_popup $headctxmenu $x $y
 }
 
-proc cobranch {} {
-    global headmenuid headmenuhead headids
+proc checkout {newhead newheadid} {
+    global headids
     global showlocalchanges
 
     # check the tree is clean first??
-    set newhead $headmenuhead
+
+    # The ref is either the head, if it exists, or the ID
+    set newheadref [expr {$newhead ne "" ? $newhead : $newheadid}]
+
     set command [list | git checkout]
     if {[string match "remotes/*" $newhead]} {
         set remote $newhead
         set newhead [string range $newhead [expr [string last / $newhead] + 1] end]
-        # The following check is redundant - the menu option should
-        # be disabled to begin with...
         if {[info exists headids($newhead)]} {
             error_popup [mc "A local branch named %s exists already" $newhead]
             return
         }
         lappend command -b $newhead --track $remote
     } else {
-        lappend command $newhead
+        lappend command $newheadref
     }
     lappend command 2>@1
     nowbusy checkout [mc "Checking out"]
@@ -10011,11 +10014,11 @@ proc cobranch {} {
             dodiffindex
         }
     } else {
-        filerun $fd [list readcheckoutstat $fd $newhead $headmenuid]
+        filerun $fd [list readcheckoutstat $fd $newhead $newheadref $newheadid]
     }
 }
 
-proc readcheckoutstat {fd newhead newheadid} {
+proc readcheckoutstat {fd newhead newheadref newheadid} {
     global mainhead mainheadid headids idheads showlocalchanges progresscoords
     global viewmainheadid curview
 
@@ -10034,12 +10037,13 @@ proc readcheckoutstat {fd newhead newheadid} {
         return
     }
     set oldmainid $mainheadid
-    if {! [info exists headids($newhead)]} {
+
+    if {$newhead ne "" && ! [info exists headids($newhead)]} {
         set headids($newhead) $newheadid
         lappend idheads($newheadid) $newhead
         addedhead $newheadid $newhead
     }
-    set mainhead $newhead
+    set mainhead $newheadref
     set mainheadid $newheadid
     set viewmainheadid($curview) $newheadid
     redrawtags $oldmainid
-- 
gitgitgadget


^ permalink raw reply related

* [PATCH 4/9] gitk: show branch name in reset dialog
From: Jens Lidestrom via GitGitGadget @ 2023-06-27 14:41 UTC (permalink / raw)
  To: git; +Cc: Paul Mackerras [ ], Jens Lidestrom, Jens Lidestrom
In-Reply-To: <pull.1551.git.1687876884.gitgitgadget@gmail.com>

From: Jens Lidestrom <jens@lidestrom.se>

Signed-off-by: Jens Lidestrom <jens@lidestrom.se>
---
 gitk-git/gitk | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/gitk-git/gitk b/gitk-git/gitk
index 99c6fb6a848..bfe912983f4 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -1351,6 +1351,21 @@ proc selected_line_head {} {
     }
 }
 
+# If any branch is associated with the argument ID then return that. Otherwise
+# return the ID itself. Useful for displaying the best name of a commit.
+proc commit_name {id should_shorten_id} {
+    global idheads idtags
+    if {[info exists idheads($id)]} {
+        return [lindex $idheads($id) 0]
+    } elseif {[info exists idtags($id)]} {
+        return [lindex $idtags($id) 0]
+    } elseif {$should_shorten_id} {
+        return [string range $id 0 7]
+    } else {
+        return $id
+    }
+}
+
 proc closevarcs {v} {
     global varctok varccommits varcid parents children
     global cmitlisted commitidx vtokmod curview numcommits
@@ -9875,7 +9890,7 @@ proc resethead {reset_target_id} {
     make_transient $w .
     wm title $w [mc "Confirm reset"]
     ${NS}::label $w.m -text \
-        [mc "Reset branch %s to %s?" $mainhead [string range $reset_target_id 0 7]]
+        [mc "Reset branch %s to %s?" $mainhead [commit_name $reset_target_id 1]]
     pack $w.m -side top -fill x -padx 20 -pady 20
     ${NS}::labelframe $w.f -text [mc "Reset type:"]
     set resettype mixed
-- 
gitgitgadget


^ permalink raw reply related

* [PATCH 3/9] gitk: add keyboard bind for reset
From: Jens Lidestrom via GitGitGadget @ 2023-06-27 14:41 UTC (permalink / raw)
  To: git; +Cc: Paul Mackerras [ ], Jens Lidestrom, Jens Lidestrom
In-Reply-To: <pull.1551.git.1687876884.gitgitgadget@gmail.com>

From: Jens Lidestrom <jens@lidestrom.se>

Signed-off-by: Jens Lidestrom <jens@lidestrom.se>
---
 gitk-git/gitk | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/gitk-git/gitk b/gitk-git/gitk
index 574a80fbcc2..99c6fb6a848 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -2675,6 +2675,7 @@ proc makewindow {} {
     bind $ctext $ctxbut {pop_diff_menu %W %X %Y %x %y}
     bind $ctext <Button-1> {focus %W}
     bind $ctext <<Selection>> rehighlight_search_results
+    bind . <$M1B-t> {resethead [selected_line_id]}
     for {set i 1} {$i < 10} {incr i} {
         bind . <$M1B-Key-$i> [list go_to_parent $i]
     }
@@ -2693,7 +2694,7 @@ proc makewindow {} {
         {mc "Write commit to file" command writecommit}
         {mc "Create new branch" command mkbranch}
         {mc "Cherry-pick this commit" command cherrypick}
-        {mc "Reset current branch to here" command resethead}
+        {mc "Reset current branch to here" command {resethead $rowmenuid}}
         {mc "Mark this commit" command markhere}
         {mc "Return to mark" command gotomark}
         {mc "Find descendant of this and mark" command find_common_desc}
@@ -3166,6 +3167,7 @@ proc keys {} {
 [mc "<%s-KP->	Decrease font size" $M1T]
 [mc "<%s-minus>	Decrease font size" $M1T]
 [mc "<F5>		Update"]
+[mc "<%s-T>		Reset current branch to selected commit" $M1T]
 " \
             -justify left -bg $bgcolor -border 2 -relief groove
     pack $w.m -side top -fill both -padx 2 -pady 2
@@ -9859,8 +9861,13 @@ proc revert {} {
     notbusy revert
 }
 
-proc resethead {} {
-    global mainhead rowmenuid confirm_ok resettype NS
+proc resethead {reset_target_id} {
+    global headids mainhead confirm_ok resettype NS
+
+    if {! [info exists headids($mainhead)]} {
+        error_popup [mc "Cannot reset a detached head"]
+        return
+    }
 
     set confirm_ok 0
     set w ".confirmreset"
@@ -9868,7 +9875,7 @@ proc resethead {} {
     make_transient $w .
     wm title $w [mc "Confirm reset"]
     ${NS}::label $w.m -text \
-        [mc "Reset branch %s to %s?" $mainhead [string range $rowmenuid 0 7]]
+        [mc "Reset branch %s to %s?" $mainhead [string range $reset_target_id 0 7]]
     pack $w.m -side top -fill x -padx 20 -pady 20
     ${NS}::labelframe $w.f -text [mc "Reset type:"]
     set resettype mixed
@@ -9891,13 +9898,13 @@ proc resethead {} {
     tkwait window $w
     if {!$confirm_ok} return
     if {[catch {set fd [open \
-            [list | git reset --$resettype $rowmenuid 2>@1] r]} err]} {
+            [list | git reset --$resettype $reset_target_id 2>@1] r]} err]} {
         error_popup $err
     } else {
         dohidelocalchanges
         filerun $fd [list readresetstat $fd]
         nowbusy reset [mc "Resetting"]
-        selbyid $rowmenuid
+        selbyid $reset_target_id
     }
 }
 
-- 
gitgitgadget


^ permalink raw reply related

* [PATCH 2/9] gitk: use term "current branch" in gui
From: Jens Lidestrom via GitGitGadget @ 2023-06-27 14:41 UTC (permalink / raw)
  To: git; +Cc: Paul Mackerras [ ], Jens Lidestrom, Jens Lidestrom
In-Reply-To: <pull.1551.git.1687876884.gitgitgadget@gmail.com>

From: Jens Lidestrom <jens@lidestrom.se>

This is the term that is used in the official documentation. Instead of
"HEAD branch" which in not standard.

Signed-off-by: Jens Lidestrom <jens@lidestrom.se>
---
 gitk-git/gitk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gitk-git/gitk b/gitk-git/gitk
index a533ff9002e..574a80fbcc2 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -2693,7 +2693,7 @@ proc makewindow {} {
         {mc "Write commit to file" command writecommit}
         {mc "Create new branch" command mkbranch}
         {mc "Cherry-pick this commit" command cherrypick}
-        {mc "Reset HEAD branch to here" command resethead}
+        {mc "Reset current branch to here" command resethead}
         {mc "Mark this commit" command markhere}
         {mc "Return to mark" command gotomark}
         {mc "Find descendant of this and mark" command find_common_desc}
-- 
gitgitgadget


^ permalink raw reply related

* [PATCH 1/9] gitk: add procedures to get commit info from selected row
From: Jens Lidestrom via GitGitGadget @ 2023-06-27 14:41 UTC (permalink / raw)
  To: git; +Cc: Paul Mackerras [ ], Jens Lidestrom, Jens Lidestrom
In-Reply-To: <pull.1551.git.1687876884.gitgitgadget@gmail.com>

From: Jens Lidestrom <jens@lidestrom.se>

These procedures are useful for implementing keyboard commands. Keyboard
commands don't have access to commits that are selected by the mouse and
hence must get info from the selected row.

Signed-off-by: Jens Lidestrom <jens@lidestrom.se>
---
 gitk-git/gitk | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/gitk-git/gitk b/gitk-git/gitk
index df3ba2ea99b..a533ff9002e 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -1333,6 +1333,24 @@ proc commitonrow {row} {
     return $id
 }
 
+# Get commits ID of the row that is selected in the GUI
+proc selected_line_id {} {
+    global selectedline
+    return [commitonrow $selectedline]
+}
+
+# Gets the first branch name of the row that is selected in the GUI, or the
+# empty string if there is no branches on that commit.
+proc selected_line_head {} {
+    global idheads
+    set id [selected_line_id]
+    if {[info exists idheads($id)]} {
+        return [lindex $idheads($id) 0]
+    } else {
+        return ""
+    }
+}
+
 proc closevarcs {v} {
     global varctok varccommits varcid parents children
     global cmitlisted commitidx vtokmod curview numcommits
-- 
gitgitgadget


^ permalink raw reply related

* [PATCH 0/9] gitk: improve keyboard support
From: Jens Lidestrom via GitGitGadget @ 2023-06-27 14:41 UTC (permalink / raw)
  To: git; +Cc: Paul Mackerras [ ], Jens Lidestrom

It is often convenient to use the keyboard to navigate the gitk GUI and
there are keyboard shortcut bindings for many operations such as searching
and scrolling. There is however no keyboard binding for the most common
operations on branches and commits: Check out, reset, cherry-pick, create
and delete branches.

This PR adds keyboard bindings for these 5 commands. It also adjusts some
GUI focus defaults to simplify keyboard navigation.

Some refactoring of the command implementation has been necessary.
Originally the commands was using the mouse context menu to get info about
the head and commit to act on. When using keyboard binds this information
isn't available so instead the row that is selected in the GUI is used. By
adding procedures for doing this the PR lays the groundwork for more similar
keyboard binds in the future.

I'm including Paul Mackerras because he seems to be the maintainer of gitk.
Can you review, Paul?

Jens Lidestrom (9):
  gitk: add procedures to get commit info from selected row
  gitk: use term "current branch" in gui
  gitk: add keyboard bind for reset
  gitk: show branch name in reset dialog
  gitk: add keyboard bind for checkout
  gitk: add keyboard bind for create and remove branch
  gitk: add keyboard bind to cherry-pick
  gitk: focus ok button in reset dialog
  gitk: default select reset hard in dialog

 gitk-git/gitk | 132 ++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 96 insertions(+), 36 deletions(-)


base-commit: 94486b6763c29144c60932829a65fec0597e17b3
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1551%2Fjensli%2Fkeyboard-for-gitk-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1551/jensli/keyboard-for-gitk-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1551
-- 
gitgitgadget

^ permalink raw reply

* [PATCH 3/3] diff --no-index: support reading from named pipes
From: Phillip Wood @ 2023-06-27 14:10 UTC (permalink / raw)
  To: git
  Cc: Jeff King, Taylor Blau, brian m. carlson, Thomas Guyot-Sionnest,
	Junio C Hamano, Phillip Wood
In-Reply-To: <cover.1687874975.git.phillip.wood@dunelm.org.uk>

From: Phillip Wood <phillip.wood@dunelm.org.uk>

In some shells, such as bash and zsh, it's possible to use a command
substitution to provide the output of a command as a file argument to
another process, like so:

  diff -u <(printf "a\nb\n") <(printf "a\nc\n")

However, this syntax does not produce useful results with "git diff
--no-index". On macOS, the arguments to the command are named pipes
under /dev/fd, and git diff doesn't know how to handle a named pipe. On
Linux, the arguments are symlinks to pipes, so git diff "helpfully"
diffs these symlinks, comparing their targets like "pipe:[1234]" and
"pipe:[5678]".

To address this "diff --no-index" is changed so that if a path given on
the commandline is a named pipe or a symbolic link that resolves to a
named pipe then we read the data to diff from that pipe. This is
implemented by generalizing the code that already exists to handle
reading from stdin when the user passes the path "-".

As process substitution is not support by POSIX this change is tested by
using a pipe and a symbolic link to a pipe.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
 diff-no-index.c          | 80 ++++++++++++++++++++++++----------------
 t/t4053-diff-no-index.sh | 25 +++++++++++++
 2 files changed, 73 insertions(+), 32 deletions(-)

diff --git a/diff-no-index.c b/diff-no-index.c
index 7b9327f8f3..45fbd7cdbe 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -41,7 +41,7 @@ static int read_directory_contents(const char *path, struct string_list *list)
  */
 static const char file_from_standard_input[] = "-";
 
-static int get_mode(const char *path, int *mode)
+static int get_mode(const char *path, int is_pipe, int *mode)
 {
 	struct stat st;
 
@@ -51,7 +51,7 @@ static int get_mode(const char *path, int *mode)
 	else if (!strcasecmp(path, "nul"))
 		*mode = 0;
 #endif
-	else if (path == file_from_standard_input)
+	else if (is_pipe)
 		*mode = create_ce_mode(0666);
 	else if (lstat(path, &st))
 		return error("Could not access '%s'", path);
@@ -60,13 +60,18 @@ static int get_mode(const char *path, int *mode)
 	return 0;
 }
 
-static void populate_from_stdin(struct diff_filespec *s)
+static void populate_from_pipe(struct diff_filespec *s, int is_stdin)
 {
 	struct strbuf buf = STRBUF_INIT;
 	size_t size = 0;
+	int fd = 0;
 
-	if (strbuf_read(&buf, 0, 0) < 0)
+	if (!is_stdin)
+		fd = xopen(s->path, O_RDONLY);
+	if (strbuf_read(&buf, fd, 0) < 0)
 		die_errno("error while reading from stdin");
+	if (!is_stdin)
+		close(fd);
 
 	s->should_munmap = 0;
 	s->data = strbuf_detach(&buf, &size);
@@ -75,40 +80,43 @@ static void populate_from_stdin(struct diff_filespec *s)
 	s->is_stdin = 1;
 }
 
-static struct diff_filespec *noindex_filespec(const char *name, int mode)
+static struct diff_filespec *noindex_filespec(const char *name, int is_pipe,
+					      int mode)
 {
 	struct diff_filespec *s;
 
 	if (!name)
 		name = "/dev/null";
 	s = alloc_filespec(name);
 	fill_filespec(s, null_oid(), 0, mode);
-	if (name == file_from_standard_input)
-		populate_from_stdin(s);
+	if (is_pipe)
+		populate_from_pipe(s, name == file_from_standard_input);
 	return s;
 }
 
 static int queue_diff(struct diff_options *o,
-		      const char *name1, const char *name2)
+		      const char *name1, int is_pipe1,
+		      const char *name2, int is_pipe2)
 {
 	int mode1 = 0, mode2 = 0;
 
-	if (get_mode(name1, &mode1) || get_mode(name2, &mode2))
+	if (get_mode(name1, is_pipe1, &mode1) ||
+	    get_mode(name2, is_pipe2, &mode2))
 		return -1;
 
 	if (mode1 && mode2 && S_ISDIR(mode1) != S_ISDIR(mode2)) {
 		struct diff_filespec *d1, *d2;
 
 		if (S_ISDIR(mode1)) {
 			/* 2 is file that is created */
-			d1 = noindex_filespec(NULL, 0);
-			d2 = noindex_filespec(name2, mode2);
+			d1 = noindex_filespec(NULL, 0, 0);
+			d2 = noindex_filespec(name2, is_pipe2, mode2);
 			name2 = NULL;
 			mode2 = 0;
 		} else {
 			/* 1 is file that is deleted */
-			d1 = noindex_filespec(name1, mode1);
-			d2 = noindex_filespec(NULL, 0);
+			d1 = noindex_filespec(name1, is_pipe1, mode1);
+			d2 = noindex_filespec(NULL, 0, 0);
 			name1 = NULL;
 			mode1 = 0;
 		}
@@ -173,7 +181,7 @@ static int queue_diff(struct diff_options *o,
 				n2 = buffer2.buf;
 			}
 
-			ret = queue_diff(o, n1, n2);
+			ret = queue_diff(o, n1, 0, n2, 0);
 		}
 		string_list_clear(&p1, 0);
 		string_list_clear(&p2, 0);
@@ -189,8 +197,8 @@ static int queue_diff(struct diff_options *o,
 			SWAP(name1, name2);
 		}
 
-		d1 = noindex_filespec(name1, mode1);
-		d2 = noindex_filespec(name2, mode2);
+		d1 = noindex_filespec(name1, is_pipe1, mode1);
+		d2 = noindex_filespec(name2, is_pipe2, mode2);
 		diff_queue(&diff_queued_diff, d1, d2);
 		return 0;
 	}
@@ -213,18 +221,11 @@ static void append_basename(struct strbuf *path, const char *dir, const char *fi
  * Note that we append the basename of F to D/, so "diff a/b/file D"
  * becomes "diff a/b/file D/file", not "diff a/b/file D/a/b/file".
  */
-static void fixup_paths(const char **path, struct strbuf *replacement)
+static void fixup_paths(const char **path, int *is_dir, struct strbuf *replacement)
 {
-	unsigned int isdir0, isdir1;
-
-	if (path[0] == file_from_standard_input ||
-	    path[1] == file_from_standard_input)
-		return;
-	isdir0 = is_directory(path[0]);
-	isdir1 = is_directory(path[1]);
-	if (isdir0 == isdir1)
-		return;
-	if (isdir0) {
+	if (is_dir[0] == is_dir[1])
+		return;
+	if (is_dir[0]) {
 		append_basename(replacement, path[0], path[1]);
 		path[0] = replacement->buf;
 	} else {
@@ -246,6 +247,8 @@ int diff_no_index(struct rev_info *revs,
 	int ret = 1;
 	const char *paths[2];
 	char *to_free[ARRAY_SIZE(paths)] = { 0 };
+	int is_dir[ARRAY_SIZE(paths)] = { 0 };
+	int is_pipe[ARRAY_SIZE(paths)] = { 0 };
 	struct strbuf replacement = STRBUF_INIT;
 	const char *prefix = revs->prefix;
 	struct option no_index_options[] = {
@@ -267,18 +270,30 @@ int diff_no_index(struct rev_info *revs,
 	FREE_AND_NULL(options);
 	for (i = 0; i < 2; i++) {
 		const char *p = argv[i];
-		if (!strcmp(p, "-"))
+		if (!strcmp(p, "-")) {
 			/*
 			 * stdin should be spelled as "-"; if you have
 			 * path that is "-", spell it as "./-".
 			 */
 			p = file_from_standard_input;
-		else if (prefix)
-			p = to_free[i] = prefix_filename(prefix, p);
+			is_pipe[i] = 1;
+		} else {
+			struct stat st;
+
+			if (prefix)
+				p = to_free[i] = prefix_filename(prefix, p);
+			if (stat(p, &st))
+				;
+			else if (S_ISDIR(st.st_mode))
+				is_dir[i] = 1;
+			else if (S_ISFIFO(st.st_mode))
+				is_pipe[i] = 1;
+		}
 		paths[i] = p;
 	}
 
-	fixup_paths(paths, &replacement);
+	if (!is_pipe[0] && !is_pipe[1])
+		fixup_paths(paths, is_dir, &replacement);
 
 	revs->diffopt.skip_stat_unmatch = 1;
 	if (!revs->diffopt.output_format)
@@ -295,7 +310,8 @@ int diff_no_index(struct rev_info *revs,
 	setup_diff_pager(&revs->diffopt);
 	revs->diffopt.flags.exit_with_status = 1;
 
-	if (queue_diff(&revs->diffopt, paths[0], paths[1]))
+	if (queue_diff(&revs->diffopt,
+		       paths[0], is_pipe[0], paths[1], is_pipe[1]))
 		goto out;
 	diff_set_mnemonic_prefix(&revs->diffopt, "1/", "2/");
 	diffcore_std(&revs->diffopt);
diff --git a/t/t4053-diff-no-index.sh b/t/t4053-diff-no-index.sh
index d14b194ea2..30b9e2c2f0 100755
--- a/t/t4053-diff-no-index.sh
+++ b/t/t4053-diff-no-index.sh
@@ -223,4 +223,29 @@ test_expect_success "diff --no-index treats '-' as stdin" '
 	test_write_lines 1 | git diff --no-index -- a/1 - >actual &&
 	test_must_be_empty actual
 '
+
+test_expect_success PIPE,SYMLINKS 'diff --no-index reads from pipes' '
+	mkfifo old &&
+	mkfifo new &&
+	ln -s new new-link &&
+	{
+		(test_write_lines a b c >old) &
+		(test_write_lines a x c >new) &
+	} &&
+
+	cat >expect <<-EOF &&
+	diff --git a/old b/new-link
+	--- a/old
+	+++ b/new-link
+	@@ -1,3 +1,3 @@
+	 a
+	-b
+	+x
+	 c
+	EOF
+
+	test_expect_code 1 git diff --no-index old new-link >actual &&
+	test_cmp expect actual
+'
+
 test_done
-- 
2.40.1.852.g22d29fd9ba


^ permalink raw reply related

* [PATCH 2/3] t4054: test diff --no-index with stdin
From: Phillip Wood @ 2023-06-27 14:10 UTC (permalink / raw)
  To: git
  Cc: Jeff King, Taylor Blau, brian m. carlson, Thomas Guyot-Sionnest,
	Junio C Hamano, Phillip Wood
In-Reply-To: <cover.1687874975.git.phillip.wood@dunelm.org.uk>

From: Phillip Wood <phillip.wood@dunelm.org.uk>

"git diff --no-index" supports reading from stdin with the path "-".
There is no test coverage for this so add a regression test before
changing the code in the next commit.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
 t/t4053-diff-no-index.sh | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/t/t4053-diff-no-index.sh b/t/t4053-diff-no-index.sh
index 4e9fa0403d..d14b194ea2 100755
--- a/t/t4053-diff-no-index.sh
+++ b/t/t4053-diff-no-index.sh
@@ -205,4 +205,22 @@ test_expect_success POSIXPERM,SYMLINKS 'diff --no-index normalizes: mode not lik
 	test_cmp expected actual
 '
 
+test_expect_success "diff --no-index treats '-' as stdin" '
+	cat >expect <<-EOF &&
+	diff --git a/- b/a/1
+	index $ZERO_OID..$(git hash-object --stdin <a/1) 100644
+	--- a/-
+	+++ b/a/1
+	@@ -1 +1 @@
+	-x
+	+1
+	EOF
+
+	test_write_lines x | test_expect_code 1 \
+		git -c core.abbrev=no diff --no-index -- - a/1 >actual &&
+	test_cmp expect actual &&
+
+	test_write_lines 1 | git diff --no-index -- a/1 - >actual &&
+	test_must_be_empty actual
+'
 test_done
-- 
2.40.1.852.g22d29fd9ba


^ permalink raw reply related

* [PATCH 1/3] diff --no-index: die on error reading stdin
From: Phillip Wood @ 2023-06-27 14:10 UTC (permalink / raw)
  To: git
  Cc: Jeff King, Taylor Blau, brian m. carlson, Thomas Guyot-Sionnest,
	Junio C Hamano, Phillip Wood
In-Reply-To: <cover.1687874975.git.phillip.wood@dunelm.org.uk>

From: Phillip Wood <phillip.wood@dunelm.org.uk>

If there is an error when reading from stdin then "diff --no-index"
prints an error message but continues to try and diff a file named "-"
resulting in an error message that looks like

    error: error while reading from stdin: Invalid argument
    fatal: stat '-': No such file or directory

assuming that no file named "-" exists. If such a file exists it prints
the first error message and generates the diff from that file which is
not what the user wanted. Instead just die() straight away if we cannot
read from stdin.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
 diff-no-index.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/diff-no-index.c b/diff-no-index.c
index 4296940f90..7b9327f8f3 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -60,20 +60,19 @@ static int get_mode(const char *path, int *mode)
 	return 0;
 }
 
-static int populate_from_stdin(struct diff_filespec *s)
+static void populate_from_stdin(struct diff_filespec *s)
 {
 	struct strbuf buf = STRBUF_INIT;
 	size_t size = 0;
 
 	if (strbuf_read(&buf, 0, 0) < 0)
-		return error_errno("error while reading from stdin");
+		die_errno("error while reading from stdin");
 
 	s->should_munmap = 0;
 	s->data = strbuf_detach(&buf, &size);
 	s->size = size;
 	s->should_free = 1;
 	s->is_stdin = 1;
-	return 0;
 }
 
 static struct diff_filespec *noindex_filespec(const char *name, int mode)
-- 
2.40.1.852.g22d29fd9ba


^ 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