[PATCH 0/5] Default aliases

Virtually all VCS in history have default aliases, except git. Let's
fix that.

To make the aliases uncontroversial all of them have to follow certain
rules:

 1) Each default alias should have two characters
 2) Each default alias should map to a command without arguments
 3) Each default alias be widely used in the wild

The list of default aliases on this series have been discussed before,
and even Junio stated "I think it might be OK to implement them" [1].
Since git is virtually unusable without aliases, it's an imperative to
make it useful by default.

Additionally, users should be able to override the default aliases
without any issue.

[1] https://lore.kernel.org/git/xmqqtx9m8obr.fsf@gitster.dls.corp.google.com/

Felipe Contreras (5):
  test: add missing whitespaces
  config: trivial style fix
  config: trivial struct initialization cleanup
  config: initialize origin_type correctly
  config: add default aliases

 Documentation/git-branch.txt      |  4 +++
 Documentation/git-cherry-pick.txt |  4 +++
 Documentation/git-commit.txt      |  4 +++
 Documentation/git-mergetool.txt   |  4 +++
 Documentation/git-rebase.txt      |  4 +++
 Documentation/git-status.txt      |  4 +++
 config.c                          | 44 +++++++++++++++++++++++++------
 config.h                          |  3 ++-
 t/t1300-config.sh                 |  1 +
 t/test-lib.sh                     |  3 +++
 10 files changed, 66 insertions(+), 9 deletions(-)

-- 
2.32.0.94.g4574ca548c

Signed-off-by: Felipe Contreras <redacted>
---
 t/t1300-config.sh | 1 +
 t/test-lib.sh     | 1 +
 2 files changed, 2 insertions(+)

diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index 9ff46f3b04..453222b32f 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh

@@ -335,6 +335,7 @@ test_expect_success 'working --list' '
 	git config --list > output &&
 	test_cmp expect output
 '
+
 test_expect_success '--list without repo produces empty output' '
 	git --git-dir=nonexistent config --list >output &&
 	test_must_be_empty output

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 54938c6427..49b80a4eb5 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh

@@ -430,6 +430,7 @@ unset VISUAL EMAIL LANGUAGE COLUMNS $("$PERL_PATH" -e '
 	my @vars = grep(/^GIT_/ && !/^GIT_($ok)/o, @env);
 	print join("\n", @vars);
 ')
+
 unset XDG_CACHE_HOME
 unset XDG_CONFIG_HOME
 unset GITPERLLIB

-- 
2.32.0.94.g4574ca548c

Signed-off-by: Felipe Contreras <redacted>
---
 config.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/config.c b/config.c
index f9c400ad30..dc896c434e 100644
--- a/config.c
+++ b/config.c

@@ -3482,7 +3482,7 @@ const char *current_config_origin_type(void)
 	int type;
 	if (current_config_kvi)
 		type = current_config_kvi->origin_type;
-	else if(cf)
+	else if (cf)
 		type = cf->origin_type;
 	else
 		BUG("current_config_origin_type called outside config callback");

-- 
2.32.0.94.g4574ca548c

Signed-off-by: Felipe Contreras <redacted>
---
 config.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/config.c b/config.c
index dc896c434e..9172c96c54 100644
--- a/config.c
+++ b/config.c

@@ -2265,11 +2265,11 @@ int git_configset_get_pathname(struct config_set *cs, const char *key, const cha
 /* Functions use to read configuration from a repository */
 static void repo_read_config(struct repository *repo)
 {
-	struct config_options opts = { 0 };
-
-	opts.respect_includes = 1;
-	opts.commondir = repo->commondir;
-	opts.git_dir = repo->gitdir;
+	struct config_options opts = {
+		.respect_includes = 1,
+		.commondir = repo->commondir,
+		.git_dir = repo->gitdir,
+	};
 
 	if (!repo->config)
 		CALLOC_ARRAY(repo->config, 1);

-- 
2.32.0.94.g4574ca548c

cf->origin_type either is CONFIG_ORIGIN_CMDLINE, or it's something else.

Don't override that.

Signed-off-by: Felipe Contreras <redacted>
---
 config.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/config.c b/config.c
index 9172c96c54..666fb2c689 100644
--- a/config.c
+++ b/config.c

@@ -2087,13 +2087,12 @@ static int configset_add_value(struct config_set *cs, const char *key, const cha
 	if (cf->name) {
 		kv_info->filename = strintern(cf->name);
 		kv_info->linenr = cf->linenr;
-		kv_info->origin_type = cf->origin_type;
 	} else {
 		/* for values read from `git_config_from_parameters()` */
 		kv_info->filename = NULL;
 		kv_info->linenr = -1;
-		kv_info->origin_type = CONFIG_ORIGIN_CMDLINE;
 	}
+	kv_info->origin_type = cf->origin_type;
 	kv_info->scope = current_parsing_scope;
 	si->util = kv_info;
 

-- 
2.32.0.94.g4574ca548c

These are all the aliases everyone agrees are essential.

Virtually all VCS in the world have aliases, except git, so let's change
that.

Signed-off-by: Felipe Contreras <redacted>
---
 Documentation/git-branch.txt      |  4 ++++
 Documentation/git-cherry-pick.txt |  4 ++++
 Documentation/git-commit.txt      |  4 ++++
 Documentation/git-mergetool.txt   |  4 ++++
 Documentation/git-rebase.txt      |  4 ++++
 Documentation/git-status.txt      |  4 ++++
 config.c                          | 29 +++++++++++++++++++++++++++++
 config.h                          |  3 ++-
 t/test-lib.sh                     |  2 ++
 9 files changed, 57 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index 94dc9a54f2..fbf5ebd27a 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt

@@ -24,6 +24,10 @@ SYNOPSIS
 'git branch' (-d | -D) [-r] <branchname>...
 'git branch' --edit-description [<branchname>]
 
+ALIAS
+~~~~~
+'git br'
+
 DESCRIPTION
 -----------
 

diff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt
index 5d750314b2..b43b1a3a30 100644
--- a/Documentation/git-cherry-pick.txt
+++ b/Documentation/git-cherry-pick.txt

@@ -12,6 +12,10 @@ SYNOPSIS
 		  [-S[<keyid>]] <commit>...
 'git cherry-pick' (--continue | --skip | --abort | --quit)
 
+ALIAS
+~~~~~
+'git pi'
+
 DESCRIPTION
 -----------
 

diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 340c5fbb48..32b1fdba45 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt

@@ -17,6 +17,10 @@ SYNOPSIS
 	   [(--trailer <token>[(=|:)<value>])...] [-S[<keyid>]]
 	   [--] [<pathspec>...]
 
+ALIAS
+~~~~~
+'git co'
+
 DESCRIPTION
 -----------
 Create a new commit containing the current contents of the index and

diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt
index e587c7763a..59708a1f3e 100644
--- a/Documentation/git-mergetool.txt
+++ b/Documentation/git-mergetool.txt

@@ -10,6 +10,10 @@ SYNOPSIS
 [verse]
 'git mergetool' [--tool=<tool>] [-y | --[no-]prompt] [<file>...]
 
+ALIAS
+~~~~~
+'git mt'
+
 DESCRIPTION
 -----------
 

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 55af6fd24e..21f5ae9d0e 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt

@@ -14,6 +14,10 @@ SYNOPSIS
 	--root [<branch>]
 'git rebase' (--continue | --skip | --abort | --quit | --edit-todo | --show-current-patch)
 
+ALIAS
+~~~~~
+'git rb'
+
 DESCRIPTION
 -----------
 If <branch> is specified, 'git rebase' will perform an automatic

diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
index 83f38e3198..fcc89fa8d4 100644
--- a/Documentation/git-status.txt
+++ b/Documentation/git-status.txt

@@ -11,6 +11,10 @@ SYNOPSIS
 [verse]
 'git status' [<options>...] [--] [<pathspec>...]
 
+ALIAS
+~~~~~
+'git st'
+
 DESCRIPTION
 -----------
 Displays paths that have differences between the index file and the

diff --git a/config.c b/config.c
index 666fb2c689..c42f41599c 100644
--- a/config.c
+++ b/config.c

@@ -501,6 +501,30 @@ int git_config_key_is_valid(const char *key)
 	return !git_config_parse_key_1(key, NULL, NULL, 1);
 }
 
+static int git_config_default(config_fn_t fn, void *data)
+{
+	int ret = 0;
+	struct config_source source;
+
+	if (getenv("GIT_NO_DEFAULT_ALIASES"))
+		return 0;
+
+	memset(&source, 0, sizeof(source));
+	source.prev = cf;
+	source.origin_type = CONFIG_ORIGIN_DEFAULT;
+	cf = &source;
+
+	ret += fn("alias.co", "commit", data);
+	ret += fn("alias.rb", "rebase", data);
+	ret += fn("alias.st", "status", data);
+	ret += fn("alias.br", "branch", data);
+	ret += fn("alias.pi", "cherry-pick", data);
+	ret += fn("alias.mt", "mergetool", data);
+
+	cf = source.prev;
+	return ret;
+}
+
 static int config_parse_pair(const char *key, const char *value,
 			  config_fn_t fn, void *data)
 {

@@ -1897,6 +1921,9 @@ static int do_git_config_sequence(const struct config_options *opts,
 		repo_config = NULL;
 
 	current_parsing_scope = CONFIG_SCOPE_SYSTEM;
+
+	git_config_default(fn, data);
+
 	if (git_config_system() && system_config &&
 	    !access_or_die(system_config, R_OK,
 			   opts->system_gently ? ACCESS_EACCES_OK : 0))

@@ -3497,6 +3524,8 @@ const char *current_config_origin_type(void)
 		return "submodule-blob";
 	case CONFIG_ORIGIN_CMDLINE:
 		return "command line";
+	case CONFIG_ORIGIN_DEFAULT:
+		return "default";
 	default:
 		BUG("unknown config origin type");
 	}

diff --git a/config.h b/config.h
index 9038538ffd..bc3ecca313 100644
--- a/config.h
+++ b/config.h

@@ -58,7 +58,8 @@ enum config_origin_type {
 	CONFIG_ORIGIN_FILE,
 	CONFIG_ORIGIN_STDIN,
 	CONFIG_ORIGIN_SUBMODULE_BLOB,
-	CONFIG_ORIGIN_CMDLINE
+	CONFIG_ORIGIN_CMDLINE,
+	CONFIG_ORIGIN_DEFAULT
 };
 
 enum config_event_t {

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 49b80a4eb5..a15965e2f4 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh

@@ -456,6 +456,8 @@ GIT_DEFAULT_HASH="${GIT_TEST_DEFAULT_HASH:-sha1}"
 export GIT_DEFAULT_HASH
 GIT_TEST_MERGE_ALGORITHM="${GIT_TEST_MERGE_ALGORITHM:-ort}"
 export GIT_TEST_MERGE_ALGORITHM
+GIT_NO_DEFAULT_ALIASES=1
+export GIT_NO_DEFAULT_ALIASES
 
 # Tests using GIT_TRACE typically don't want <timestamp> <file>:<line> output
 GIT_TRACE_BARE=1

-- 
2.32.0.94.g4574ca548c

On Fri, Jul 02 2021, Felipe Contreras wrote:

I think for these it would be good to explicitly mention the mnemonic, e.g.:

'git br', git 'br'anch. It's pretty obvious in this case, but not all of
them. This also addresses the '"ci" or "co"' discussion downthread
somewhat, i.e. at least we'll see if we always pick the first two
letters, or if it's somewhat arbitrary.

+~~~~~
+'git pi'

I've got this this as 'git chrp' locally FWIW, I'd think this would make
more sense if it was called 'git pick'.

+~~~~~
+'git co'

Not going to wade into the downhtread co/ci discussion, except to say
that this is 'co'mmit, i.e. first two letters, like 'br'anch.

 'git mergetool' [--tool=<tool>] [-y | --[no-]prompt] [<file>...]
 
+ALIAS
+~~~~~
+'git mt'

Maybe it's just me, but I don't think I've ever used git-mergetool
directly. I don't think it's worthy of squatting on such a short name.

+ALIAS
+~~~~~
+'git rb'

So 'r'e'b'ase, not 're'base.

 'git status' [<options>...] [--] [<pathspec>...]
 
+ALIAS
+~~~~~
+'git st'

FWIW I've got this aliased to 'git status --short', anyway, 'st'atus, so
first two letters...

+static int git_config_default(config_fn_t fn, void *data)
+{
+	int ret = 0;
+	struct config_source source;
+
+	if (getenv("GIT_NO_DEFAULT_ALIASES"))
+		return 0;

Can't we just include this under GIT_TEST_DISALLOW_ABBREVIATED_OPTIONS?
Maybe rename it to GIT_TEST_DISALLOW_ABBREVIATED now that the "OPTIONS"
part is considered inaccurate.

+	memset(&source, 0, sizeof(source));
+	source.prev = cf;
+	source.origin_type = CONFIG_ORIGIN_DEFAULT;
+	cf = &source;
+
+	ret += fn("alias.co", "commit", data);
+	ret += fn("alias.rb", "rebase", data);
+	ret += fn("alias.st", "status", data);
+	ret += fn("alias.br", "branch", data);
+	ret += fn("alias.pi", "cherry-pick", data);
+	ret += fn("alias.mt", "mergetool", data);

I haven't looked but does this also inject things into the configset
API, or is it just going to be used by things that do
git_config_mycommand and fall back on git_config_default?

As long as the aliases mechanism picks it up I suppose it's fine.

Ah, this is likely it, do we incclude this in 'git config -l' etc? 

Really needs more tests.

We had some other thread where this was discussed where I suggested that
we implement some way to include default config. Ah, here it is:
https://lore.kernel.org/git/87eedj74dr.fsf@evledraar.gmail.com/

It's more work for this, but I think it would really go a long way to
addressing the concerns people are going to have about this.

I think we should not opt-in to this from day one, but have some knob to
enable including one of those shipped-by-default alias includes. Then
people could trivially mock svn/cvs or whatever their favorite VCS is,
and eventually as people vote with their feed we could pick a canonical
one.

Ævar Arnfjörð Bjarmason wrote:

On Fri, Jul 02 2021, Felipe Contreras wrote:

quoted

These are all the aliases everyone agrees are essential.

Virtually all VCS in the world have aliases, except git, so let's change
that.

Signed-off-by: Felipe Contreras <redacted>
---
 Documentation/git-branch.txt      |  4 ++++
 Documentation/git-cherry-pick.txt |  4 ++++
 Documentation/git-commit.txt      |  4 ++++
 Documentation/git-mergetool.txt   |  4 ++++
 Documentation/git-rebase.txt      |  4 ++++
 Documentation/git-status.txt      |  4 ++++
 config.c                          | 29 +++++++++++++++++++++++++++++
 config.h                          |  3 ++-
 t/test-lib.sh                     |  2 ++
 9 files changed, 57 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index 94dc9a54f2..fbf5ebd27a 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt

@@ -24,6 +24,10 @@ SYNOPSIS
 'git branch' (-d | -D) [-r] <branchname>...
 'git branch' --edit-description [<branchname>]
 
+ALIAS
+~~~~~
+'git br'

I think for these it would be good to explicitly mention the mnemonic, e.g.:

'git br', git 'br'anch. It's pretty obvious in this case, but not all of
them.

If we are on `man git-branch(1)`, `git help branch`, or
`git branch --help` I think it's pretty obvious what the alias is for.

Especially since it's right after the synopsis.

FTR all other SCM's specify the alias directly. Perhaps we could even do
'br' instead of 'git br'.

This also addresses the '"ci" or "co"' discussion downthread
somewhat, i.e. at least we'll see if we always pick the first two
letters, or if it's somewhat arbitrary.

How? What would be the mnemonic for 'ci'?

quoted

+~~~~~
+'git pi'

I've got this this as 'git chrp' locally FWIW, I'd think this would make
more sense if it was called 'git pick'.

Yeah, but we are aiming for two letters the only other good option is
'cp' which can be easily confused.

For a past discussion on this alias see [1].

quoted

+~~~~~
+'git co'

Not going to wade into the downhtread co/ci discussion, except to say
that this is 'co'mmit, i.e. first two letters, like 'br'anch.

Yeap, so it's straightforward.

quoted

 'git mergetool' [--tool=<tool>] [-y | --[no-]prompt] [<file>...]
 
+ALIAS
+~~~~~
+'git mt'

Maybe it's just me, but I don't think I've ever used git-mergetool
directly. I don't think it's worthy of squatting on such a short name.

Huh? How is a user supposed to jump from a merge failing to mergetool?
(or rebase, or cherr-pick)

quoted

+ALIAS
+~~~~~
+'git rb'

So 'r'e'b'ase, not 're'base.

I don't know if 're' makes more sense here.

quoted

 'git status' [<options>...] [--] [<pathspec>...]
 
+ALIAS
+~~~~~
+'git st'

FWIW I've got this aliased to 'git status --short', anyway, 'st'atus, so
first two letters...

Me too. Actually --short --branch.

quoted

+static int git_config_default(config_fn_t fn, void *data)
+{
+	int ret = 0;
+	struct config_source source;
+
+	if (getenv("GIT_NO_DEFAULT_ALIASES"))
+		return 0;

Can't we just include this under GIT_TEST_DISALLOW_ABBREVIATED_OPTIONS?
Maybe rename it to GIT_TEST_DISALLOW_ABBREVIATED now that the "OPTIONS"
part is considered inaccurate.

Fine by me.

quoted

+	memset(&source, 0, sizeof(source));
+	source.prev = cf;
+	source.origin_type = CONFIG_ORIGIN_DEFAULT;
+	cf = &source;
+
+	ret += fn("alias.co", "commit", data);
+	ret += fn("alias.rb", "rebase", data);
+	ret += fn("alias.st", "status", data);
+	ret += fn("alias.br", "branch", data);
+	ret += fn("alias.pi", "cherry-pick", data);
+	ret += fn("alias.mt", "mergetool", data);

I haven't looked but does this also inject things into the configset
API, or is it just going to be used by things that do
git_config_mycommand and fall back on git_config_default?

I'm not sure what you mean. But it's basically as if you have them in
a config file.

Initially I used a diffent approach but the bash completion did not pick
them up. This is as close to a config file as possible.

quoted

 static int config_parse_pair(const char *key, const char *value,
 			  config_fn_t fn, void *data)
 {

@@ -1897,6 +1921,9 @@ static int do_git_config_sequence(const struct config_options *opts,
 		repo_config = NULL;
 
 	current_parsing_scope = CONFIG_SCOPE_SYSTEM;
+
+	git_config_default(fn, data);
+
 	if (git_config_system() && system_config &&
 	    !access_or_die(system_config, R_OK,
 			   opts->system_gently ? ACCESS_EACCES_OK : 0))

@@ -3497,6 +3524,8 @@ const char *current_config_origin_type(void)
 		return "submodule-blob";
 	case CONFIG_ORIGIN_CMDLINE:
 		return "command line";
+	case CONFIG_ORIGIN_DEFAULT:
+		return "default";
 	default:
 		BUG("unknown config origin type");
 	}

Ah, this is likely it, do we incclude this in 'git config -l' etc? 

Yes. Just like all other configurations.

quoted

diff --git a/config.h b/config.h
index 9038538ffd..bc3ecca313 100644
--- a/config.h
+++ b/config.h

@@ -58,7 +58,8 @@ enum config_origin_type {
 	CONFIG_ORIGIN_FILE,
 	CONFIG_ORIGIN_STDIN,
 	CONFIG_ORIGIN_SUBMODULE_BLOB,
-	CONFIG_ORIGIN_CMDLINE
+	CONFIG_ORIGIN_CMDLINE,
+	CONFIG_ORIGIN_DEFAULT
 };
 
 enum config_event_t {

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 49b80a4eb5..a15965e2f4 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh

@@ -456,6 +456,8 @@ GIT_DEFAULT_HASH="${GIT_TEST_DEFAULT_HASH:-sha1}"
 export GIT_DEFAULT_HASH
 GIT_TEST_MERGE_ALGORITHM="${GIT_TEST_MERGE_ALGORITHM:-ort}"
 export GIT_TEST_MERGE_ALGORITHM
+GIT_NO_DEFAULT_ALIASES=1
+export GIT_NO_DEFAULT_ALIASES
 
 # Tests using GIT_TRACE typically don't want <timestamp> <file>:<line> output
 GIT_TRACE_BARE=1

Really needs more tests.

We had some other thread where this was discussed where I suggested that
we implement some way to include default config. Ah, here it is:
https://lore.kernel.org/git/87eedj74dr.fsf@evledraar.gmail.com/

It's more work for this, but I think it would really go a long way to
addressing the concerns people are going to have about this.

I think we should not opt-in to this from day one, but have some knob to
enable including one of those shipped-by-default alias includes. Then
people could trivially mock svn/cvs or whatever their favorite VCS is,
and eventually as people vote with their feed we could pick a canonical
one.

As I mentioned there the problem is where do we put that file, and how
do we distribute it.

I think it's a cleaner approach, and we should definitely try it, but
ultimately it's not going to change the fact that these aliases should
be part of the distribution, especially if they are mentioned in the man
pages. So it would just be an alternative way of hardcoding them.

[1] https://lore.kernel.org/git/20140421204506.GD5105@thunk.org/

-- 
Felipe Contreras

On 02/07/2021 23:58, Felipe Contreras wrote:

Ævar Arnfjörð Bjarmason wrote:

quoted

quoted

+ALIAS
+~~~~~
+'git rb'

So 'r'e'b'ase, not 're'base.

I don't know if 're' makes more sense here.

re:
restore
rebase
reset

And restore is on the level of checkout => so more important.

martin wrote:

On 02/07/2021 23:58, Felipe Contreras wrote:

quoted

Ævar Arnfjörð Bjarmason wrote:

quoted

quoted

+ALIAS
+~~~~~
+'git rb'

So 'r'e'b'ase, not 're'base.

I don't know if 're' makes more sense here.

re:
restore
rebase
reset

And restore is on the level of checkout => so more important.

Right. Although we don't need to have aliases for all of them it's good
to be consistent, so perhaps:

  rb => rebase
  rs => reset
  rt => restore

I don't use restore (yet), but it's probably the one most people would
use most regularly, so maybe 're' instead of 'rt'.

-- 
Felipe Contreras

On Fri, Jul 02, 2021 at 05:05:06AM -0500, Felipe Contreras wrote:

These are all the aliases everyone agrees are essential.

Virtually all VCS in the world have aliases, except git, so let's change
that.

For anyone reviewing or discussing, here's an older thread on the same
topic:

  https://lore.kernel.org/git/1379791221-29925-1-git-send-email-felipe.contreras@gmail.com/

(I don't mean to imply that we can't revisit old decisions; but some of
the thoughts there are worth considering as input).

-Peff

On July 2, 2021 6:38 PM, martin wrote:

To: Felipe Contreras <redacted>; Ævar Arnfjörð Bjarmason <redacted>
Cc: git@vger.kernel.org; Junio C Hamano <redacted>
Subject: Re: [PATCH 5/5] config: add default aliases

On 02/07/2021 23:58, Felipe Contreras wrote:

quoted

Ævar Arnfjörð Bjarmason wrote:

quoted

quoted

+ALIAS
+~~~~~
+'git rb'

So 'r'e'b'ase, not 're'base.

I don't know if 're' makes more sense here.

re:
restore
rebase
reset

And restore is on the level of checkout => so more important.

I do not want anything helping out the use of rebase, which we actively discourage in our shop - except for rebase --autosquash to fix up topic branches for delivery. git 're' is certainly not helpful.

From an earlier suggestion, why not just put all of your desired aliases in its own file somewhere and reference them through a construct in .gitconfig like:

include="/path/to/alias-config"

which would have to be implemented, but that decouples alias definitions from core git code and allows sharing of the definitions by a team without impinging on anyone else. I have great trepidation that users are going to start writing scripts using these aliases. I am going to be implementing a team standards document that would cause any use of aliases in scripts to fail code reviews - in fact, I'm looking to implement a commit hook that rejects the use of aliases in scripts that are committed.

On July 5, 2021 10:03 AM, I wrote:

On July 2, 2021 6:38 PM, martin wrote:

quoted

To: Felipe Contreras <redacted>; Ævar Arnfjörð Bjarmason <redacted>
Cc: git@vger.kernel.org; Junio C Hamano <redacted>
Subject: Re: [PATCH 5/5] config: add default aliases

On 02/07/2021 23:58, Felipe Contreras wrote:

quoted

Ævar Arnfjörð Bjarmason wrote:

quoted

quoted

+ALIAS
+~~~~~
+'git rb'

So 'r'e'b'ase, not 're'base.

I don't know if 're' makes more sense here.

re:
restore
rebase
reset

And restore is on the level of checkout => so more important.

I do not want anything helping out the use of rebase, which we actively discourage in our shop - except for rebase --autosquash to fix up
topic branches for delivery. git 're' is certainly not helpful.

quoted

From an earlier suggestion, why not just put all of your desired aliases in its own file somewhere and reference them through a construct

in .gitconfig like:

include="/path/to/alias-config"

which would have to be implemented, but that decouples alias definitions from core git code and allows sharing of the definitions by a
team without impinging on anyone else. I have great trepidation that users are going to start writing scripts using these aliases. I am
going to be implementing a team standards document that would cause any use of aliases in scripts to fail code reviews - in fact, I'm
looking to implement a commit hook that rejects the use of aliases in scripts that are committed.

This is already in place in .gitconfig:

[include]
	path = /path/to/git-aliases

So whatever a team's alias set needs to be can be completely decoupled from git and put into its own repo, and delivered to the team that way. I'm going to recommend that my team uses this for alias management instead of this patch set.

-Randall

Jeff King wrote:

On Fri, Jul 02, 2021 at 05:05:06AM -0500, Felipe Contreras wrote:

quoted

These are all the aliases everyone agrees are essential.

Virtually all VCS in the world have aliases, except git, so let's change
that.

For anyone reviewing or discussing, here's an older thread on the same
topic:

  https://lore.kernel.org/git/1379791221-29925-1-git-send-email-felipe.contreras@gmail.com/

(I don't mean to imply that we can't revisit old decisions; but some of
the thoughts there are worth considering as input).

Re-reading that thread--and filtering all the noise--the two thoughts
that I think are worth considering are:

 1. A default alias might leak into some unofficial documentation, and
    people with a different alias could be surprised after typing that
    command and finding out it does a different thing.

 2. A person might be used to an alias doing one thing, move to a
    different machine, and be surprised that the default alias does a
    diffrent thing.

But as mentioned in that thread those two are *existing* issues. People
using certain configurations (not even aliases) are surprised when the
same command does a different thing. And also people use their aliases
in unofficial documentation already.

Default aliases would in fact make the situation less worse because if
one of these aliases leaks into unofficial documentation, there's a
higher chance that the command will do what was intended.

The counter-arguments were not addressed, so the conclussion is that
default aliases would *not* make the existing problems worse.


That being said, there's ways to mitigate these problems, for example we
could add an avdice stating that a default alias is currently being
used, something like:

  hint: You are using a default alias: co -> checkout.
  hint:
  hint: If you want to incorporate this alias into your personal
  hint: aliases, type:
  hint:
  hint:  git config --global alias.co checkout
  hint:
  hint: Disable this message with "git config advice.defaultaliases false"

There's many other ways to mitigate the issues. It would be in the best
inerest of the probject to explore all these possibilities to their full
extent instead of just throwing the towel and stay in the current
undesirable state.

Cheers.

-- 
Felipe Contreras

Randall S. Becker wrote:

On July 2, 2021 6:38 PM, martin wrote:

quoted

To: Felipe Contreras <redacted>; Ævar Arnfjörð Bjarmason <redacted>
Cc: git@vger.kernel.org; Junio C Hamano <redacted>
Subject: Re: [PATCH 5/5] config: add default aliases

On 02/07/2021 23:58, Felipe Contreras wrote:

quoted

Ævar Arnfjörð Bjarmason wrote:

quoted

quoted

+ALIAS
+~~~~~
+'git rb'

So 'r'e'b'ase, not 're'base.

I don't know if 're' makes more sense here.

re:
restore
rebase
reset

And restore is on the level of checkout => so more important.

I do not want anything helping out the use of rebase, which we actively discourage in our shop

That is a problem specific for your shop.

The defaults are meant for the majority of users. If a minority of users
(who happen to be working under the same umbrella) have a problem with
the defaults, they can change the defaults.

-- 
Felipe Contreras

On 05/07/2021 15:02, Randall S. Becker wrote:

I do not want anything helping out the use of rebase, which we  in our shop - except for rebase --autosquash to fix up topic branches for delivery.

I was wondering what the background/context to the 'actively discourage'
is? 

I'd have expected that some in-place rework (i.e. rebase) could happen
before code review, with possible further rework beyond simple
fixup/squash commits being possible after review (if demanded), but with
the same fork-point (rather than following movements in the 'upstream'),
rather similar to Git's development. i.e. Is it that the fork-point
shouldn't be moved without good reason and permission, or something else?

just wondering...

--
Philip