Johannes Schindelin <johannes.schindelin@xxxxxx> writes: > Instead of discovering the .git/ directory, read the config and then > trying to painstakingly reset all the global state if we did not find a > matching alias, let's use the early config machinery instead. s/read/&ing/, I think. My reading hiccupped while trying to figure out what two alternative approaches are being compared. > It may look like unnecessary work to discover the .git/ directory in the > early config machinery and then call setup_git_directory_gently() in the > case of a shell alias, repeating the very same discovery *again*. > However, we have to do this as the early config machinery takes pains > *not* to touch any global state, while shell aliases expect a possibly > changed working directory and at least the GIT_PREFIX and GIT_DIR > variables to be set. Makes sense. Nicely explained. > Also, one might be tempted to streamline the code in alias_lookup() to > *not* use a strbuf for the key. However, if the config reports an error, > it is far superior to tell the user that the `alias.xyz` key had a > problem than to claim that it was the `xyz` key. The mention of "streamline" is puzzling to me. When we are trying "git xyz", "alias.xyz" is the key we would look up, not "xyz"; it is not clear to anybody who didn't read the discussion on v2 (which includes the readers of "git log" in a few months) what kind of flawed streamlining could look up "xyz" and result in a bad configuration reported on it. > alias.c | 31 ++++++++++++++++++++++++------- > git.c | 55 ++++--------------------------------------------------- > t/t7006-pager.sh | 2 +- > 3 files changed, 29 insertions(+), 59 deletions(-) Happy to see the deletion of all the save/restore-env stuff. Except for the puzzlement in one paragraph in the log, looks very good. Thanks for a pleasant reading.