git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 5/5] config: add default aliases

From
Felipe Contreras <felipe.contreras@gmail.com>
Date
Jul 2, 2021, 21:58 UTC
Message-ID
<60df8c20e8518_28bb20846@natae.notmuch>
In-Reply-To
<8735sxaqln.fsf@evledraar.gmail.com>
Ævar Arnfjörð Bjarmason wrote:
Show 37 quoted lines
> 
> On Fri, Jul 02 2021, 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.
> >
> > Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> > ---
> >  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'?
Show 5 quoted lines
> > +~~~~~
> > +'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].
Show 5 quoted lines
> > +~~~~~
> > +'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.
Show 8 quoted lines
> >  '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)

Show 5 quoted lines
> > +ALIAS
> > +~~~~~
> > +'git rb'
> 
> So 'r'e'b'ase, not 're'base.
I don't know if 're' makes more sense here.
Show 8 quoted lines
> >  '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.
Show 11 quoted lines
> > +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.
Show 15 quoted lines
> > +	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.

Show 24 quoted lines
> >  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.
Show 42 quoted lines
> > 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
Previous: Ævar Arnfjörð BjarmasonNext: martin
Message 33 of 41 in “Default aliases”
  1. 0/5 Default aliasesFelipe Contreras, Jul 2, 2021
  2. 1/5 test: add missing whitespacesFelipe Contreras, Jul 2, 2021
  3. 2/5 config: trivial style fixFelipe Contreras, Jul 2, 2021
  4. 3/5 config: trivial struct initialization cleanupFelipe Contreras, Jul 2, 2021
  5. 4/5 config: initialize origin_type correctlyFelipe Contreras, Jul 2, 2021
  6. 5/5 config: add default aliasesFelipe Contreras, Jul 2, 2021
  7. Andreas SchwabJul 2, 2021
  8. Felipe ContrerasJul 2, 2021
  9. Andreas SchwabJul 2, 2021
  10. Felipe ContrerasJul 2, 2021
  11. Andreas SchwabJul 2, 2021
  12. martinJul 2, 2021
  13. Felipe ContrerasJul 2, 2021
  14. martinJul 2, 2021
  15. Randall S. BeckerJul 2, 2021
  16. Ævar Arnfjörð BjarmasonJul 2, 2021
  17. Randall S. BeckerJul 2, 2021
  18. martinJul 2, 2021
  19. Felipe ContrerasJul 2, 2021
  20. Randall S. BeckerJul 2, 2021
  21. Felipe ContrerasJul 2, 2021
  22. Junio C HamanoJul 2, 2021
  23. Felipe ContrerasJul 2, 2021
  24. Felipe ContrerasJul 2, 2021
  25. martinJul 2, 2021
  26. Felipe ContrerasJul 2, 2021
  27. Randall S. BeckerJul 2, 2021
  28. Felipe ContrerasJul 2, 2021
  29. Felipe ContrerasJul 2, 2021
  30. martinJul 2, 2021
  31. Felipe ContrerasJul 2, 2021
  32. Ævar Arnfjörð BjarmasonJul 2, 2021
  33. Felipe ContrerasJul 2, 2021
  34. martinJul 2, 2021
  35. Felipe ContrerasJul 2, 2021
  36. Randall S. BeckerJul 5, 2021
  37. Randall S. BeckerJul 6, 2021
  38. Felipe ContrerasJul 6, 2021
  39. Philip OakleyJul 10, 2021
  40. Jeff KingJul 3, 2021
  41. Felipe ContrerasJul 6, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.