Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

completions/git: bump to v2.34.1 #277

Merged
merged 8 commits into from
Feb 7, 2022

Conversation

jeansebastienh
Copy link
Contributor

@jeansebastienh jeansebastienh commented Dec 29, 2021

Hi,

Should close #203 #207 and also missing completions for git switch and some others...

wget
https://raw.githubusercontent.com/git/git/v2.34.1/contrib/completion/git-completion.bash
--output-document=completions/git.completion.sh

Edit by @akinomyoga

Fix #65 / Fix #203 / Fix #207 / Close #100

@akinomyoga
Copy link
Contributor

Thank you for your contribution! #100 (#65) and #102 are also related. Also, what I wanted to check was whether there are any custom edits by ourselves on git.completion.sh.

@akinomyoga
Copy link
Contributor

The file was first introduced in oh-my-bash by commit e65c390. There was a modification in commit f23792a.

$ cd oh-my-bash
$ git ls-tree -r e65c390b completion/git.completion.sh
100644 blob e3918c87e3adf32a9d7a4f0320c92c497376b9b5    completion/git.completion.sh
$ git ls-tree -r f23792a7 completions/git.completion.sh
100644 blob e83de04522151c15555f1ebd7f8a90ae2e4bf04a    completions/git.completion.sh

I've identified that the initial version added in oh-my-bash is identical with 8716bdca:contrib/completion/git-completion.bash:

$ cd git
$ git ls-tree -r 8716bdca contrib/completion/git-completion.bash
100644 blob e3918c87e3adf32a9d7a4f0320c92c497376b9b5    contrib/completion/git-completion.bash

So, the only change by us is f23792a. It seems the corresponding section was actually encapsulated in the main git command as git --list-cmds=main,others,alias,nohelpers and simplified in the latest version of git-completion.bash.

Conclusion: We can just import the upstream git-completion.bash without any changes.

(My struggles looking for the blob in git/git)

I tried to look up the same blob in https://github.com/git/git by looking at the history of contrib/completion/git-completion.bash but couldn't find the matching blob:

$ git ls-tree -r 6357d9d00405ae16d63610ec2d049e0ca7e391ea contrib/completion/git-completion.bash
100644 blob 539d7f84f3d84adb6d3cbb3301dcbb749033b610    contrib/completion/git-completion.bash
$ git ls-tree -r f805a00a396ee91599902cebe55620b2a4c813b9 contrib/completion/git-completion.bash
100644 blob a331ccc556b011f3e21b8df5aa49ef1bd8481d5b    contrib/completion/git-completion.bash
$ git ls-tree -r 78236550824fb2d0b150f2992589a26e89c0f1c9 contrib/completion/git-completion.bash
100644 blob 0e16f017a4144220043d27e0c1556a6d1edbdf01    contrib/completion/git-completion.bash
$ git ls-tree -r eb37527ab0b6e9ff1cbd01aa20a4dd6aa4150a96 contrib/completion/git-completion.bash
100644 blob d93441747523ab2971e114cd6a02d56b79ba3382    contrib/completion/git-completion.bash
$ git ls-tree -r 5453b83bdf92efbfe9e9aaa54d4c62ce48ced2f5 contrib/completion/git-completion.bash
100644 blob 29496353a462ba7e5f6b2de82d1a61bbe261729b    contrib/completion/git-completion.bash
$ git ls-tree -r 9743f18f3fef0b77b8715cba256a740a7238f761 contrib/completion/git-completion.bash
100644 blob 48a2f26622d992b89462edb3658b5cdccee806bc    contrib/completion/git-completion.bash
$ git ls-tree -r e0538abaf7ad330ab639f418e9de82ef8adbe5bb contrib/completion/git-completion.bash
100644 blob ba7d8dddc975a6fe2adec333fb63c954ebddd97e    contrib/completion/git-completion.bash
$ git ls-tree -r 5ecbaaf10172c75d6e398abd510a92ad69f7e015 contrib/completion/git-completion.bash
100644 blob 3f012491165ea2814fa2927d6b7942aeae9c8509    contrib/completion/git-completion.bash
$ git ls-tree -r 12435b377f4f2e49340be3361fcc8ee128337638 contrib/completion/git-completion.bash
100644 blob 15b40f877489ff43714e9cfce87f06543891b8f6    contrib/completion/git-completion.bash
$ git ls-tree -r d78d237bba5f4f65832e8f75a147cb81c59885ce contrib/completion/git-completion.bash
100644 blob d18d30b7775c89efea7f43b88aeab0073bfed2f5    contrib/completion/git-completion.bash
$ git ls-tree -r 6ecef7379ce428ee3da215eaca86b518fce80d68 contrib/completion/git-completion.bash
100644 blob 98617d30cdbffed342264f5694f78808a687d198    contrib/completion/git-completion.bash
$ git ls-tree -r fd1552d59d2f181019a8cf18582e89167be51e01 contrib/completion/git-completion.bash
100644 blob 087ea945bdfa3e146697c3efadf7f13e9ecf85ef    contrib/completion/git-completion.bash
$ git ls-tree -r e8dec567705ebeaaff70bb29bd9f724944c5ae90 contrib/completion/git-completion.bash
100644 blob 6150790387ad0fee095748cd0ad657098a8019bb    contrib/completion/git-completion.bash
$ git ls-tree -r f254eab2e024edec8b8d4a342252efba74901fdb contrib/completion/git-completion.bash
100644 blob 3e6ddaa814a5c536d5110f043b3ac6472f43d0a7    contrib/completion/git-completion.bash
$ git ls-tree -r 194280427d2f2f4ab564aae4c7aa7fa9f7e78f26 contrib/completion/git-completion.bash
100644 blob 94ed7a93b95a183f81f85fc6a91d3100fef83cc2    contrib/completion/git-completion.bash

I gave up searching by hand. I found the --follow option of git log so used it to automate the listing of hashes of the blobs. The result was nothing, i.e., I couldn't find the completion file originally added to oh-my-bash in the history of git/git using this method.

$ git log --pretty=%h --follow -- contrib/completion/git-completion.bash | while read -r hash; do
  echo "commit $hash: $(git ls-tree -r $hash contrib/completion/git-completion.bash)"
done | grep e3918c87e3adf32a9d7a4f0320c92c497376b9b5

Then, I next tried to find the closest one based on the file size.

[oh-my-bash]$ git cat-file -p e65c390b:completion/git.completion.sh | wc
   2776    6407   57446

The closest is somewhere around here:

[git]$ git log --pretty=%h --follow -- contrib/completion/git-completion.bash | while read -r line; do
  echo "commit $line: size $(git cat-file -p $line:contrib/completion/git-completion.bash | wc)"
done | less
[...]
commit 77d21f29ea: size    2922    6748   60622
commit c261a87e70: size    2921    6748   60620
commit 9512177b68: size    2776    6409   57460
commit 49416ad22a: size    2779    6420   57560
commit ac76fd54a8: size    2796    6443   57964
commit 2703c22fc2: size    2777    6414   57544
commit 6bc6b6c0dc: size    2778    6412   57505
commit 5f072e0017: size    2744    6328   56682
commit 6d308627ca: size    2803    6458   57927
commit 58142c09a4: size    2803    6457   57920
commit 634d2344e6: size    2872    6651   59519
commit 7c599e92aa: size    2822    6552   58444
commit 21d2a9e3cc: size    2778    6408   57504
commit b462c02402: size    2800    6452   57873
commit d3bfbf91df: size    2752    6334   56609
commit 4d10b7e1bc: size    2774    6406   57418
commit 716b29db91: size    2774    6404   57399
commit 59305aeeba: size    2770    6392   57244
commit 435ec090ec: size    2759    6359   56990
commit d7d4ca87a9: size    2770    6392   57244
commit f7c2e1a042: size    2759    6363   57021
commit fa4b5e3a35: size    2763    6371   57122
commit e6414b4645: size    2746    6325   56577
commit 17c4ddbbaf: size    2759    6364   57029
commit 89f09dd34e: size    2743    6324   56631
commit ccab28a947: size    2758    6358   56968
commit 160fcdb007: size    2752    6337   56632
commit dfbe5eeb32: size    2758    6360   56974
commit 01861cb7a2: size    2747    6325   56576
commit 905f2036d0: size    2733    6295   56365
commit 578625fa91: size    2742    6310   56450
commit 09bb6520d4: size    2749    6336   56670
commit 72dbb36554: size    2702    6158   55281
commit 956352b67e: size    2756    6345   56694
commit 12bdc880c7: size    2740    6309   56421
commit e8f9e42829: size    2744    6315   56464
commit f6f2a9e42d: size    2751    6330   56569
commit a074aa90a8: size    2702    6156   55265
[...]

I have taken several diffs to find out that the closest seems to be 9512177b68:contrib/completion/git-completion.bash

diff -bwu <(git cat-file -p 9512177b68:contrib/completion/git-completion.bash) ohmybash-completion.bash
--- /dev/fd/63  2021-12-29 18:01:32.998455269 +0900
+++ ohmybash-completion.bash    2021-12-29 17:29:53.722799453 +0900
@@ -1670,10 +1670,10 @@
 {
        local dir="$(__gitdir)"
        if [ -f "$dir"/rebase-merge/interactive ]; then
-               __gitcomp "--continue --skip --abort --quit --edit-todo"
+               __gitcomp "--continue --skip --abort --edit-todo"
                return
        elif [ -d "$dir"/rebase-apply ] || [ -d "$dir"/rebase-merge ]; then
-               __gitcomp "--continue --skip --abort --quit"
+               __gitcomp "--continue --skip --abort"
                return
        fi
        __git_complete_strategy && return

Actually, the commit 9512177b68 seems to be exactly the reverse of the above diff, so I tried to look up the previous commit and found that 8716bdca:contrib/completion/git-completion.bash is actually identical to the oh-my-bash version.

I don't know why this commit 8716bdca has not been listed by git log --follow. Then, I found that git log --follow actually doesn't include merge commits, so we need to also specify the -m option.

I finally figured out how to find the blob in the history:

$ git log -m --pretty=%h --follow -- contrib/completion/git-completion.bash | while read -r line; do
  echo "commit $line: $(git ls-tree -r $line contrib/completion/git-completion.bash)"
done | grep e3918c87e3adf32a9d7a4f0320c92c497376b9b5
[...]
commit 8020803f50: 100644 blob e3918c87e3adf32a9d7a4f0320c92c497376b9b5 contrib/completion/git-completion.bash
commit e6a6a768ca: 100644 blob e3918c87e3adf32a9d7a4f0320c92c497376b9b5 contrib/completion/git-completion.bash
commit 70bd996071: 100644 blob e3918c87e3adf32a9d7a4f0320c92c497376b9b5 contrib/completion/git-completion.bash
commit 4091558cfe: 100644 blob e3918c87e3adf32a9d7a4f0320c92c497376b9b5 contrib/completion/git-completion.bash
commit 895f20de9e: 100644 blob e3918c87e3adf32a9d7a4f0320c92c497376b9b5 contrib/completion/git-completion.bash
commit d7145ef275: 100644 blob e3918c87e3adf32a9d7a4f0320c92c497376b9b5 contrib/completion/git-completion.bash
commit d4bd6781de: 100644 blob e3918c87e3adf32a9d7a4f0320c92c497376b9b5 contrib/completion/git-completion.bash
commit 8716bdca26: 100644 blob e3918c87e3adf32a9d7a4f0320c92c497376b9b5 contrib/completion/git-completion.bash
commit 8716bdca26: 100644 blob e3918c87e3adf32a9d7a4f0320c92c497376b9b5 contrib/completion/git-completion.bash

@akinomyoga
Copy link
Contributor

Should close #203 #207 and also missing completions for git switch and some others...

I've checked #203 and #207.

I'm thinking that these issues are actually caused by the inconsistency between the version of git installed on the user's system and the version of git-completion.bash bundled with oh-my-bash. So, actually, the problem wouldn't be perfectly solved just by importing the latest version of git-completion.bash because git in the user's system is not the latest development version of git. There are still inconsistencies.

Actually, the completion script for the user's git version is supposed to be placed in $prefix/share/bash-completion/completions/git or in $prefix/share/git-core/contrib/completion/git-completion.bash where $prefix is the install location of git typically being /usr. I believe we should basically load these files rather than including a certain version of git-completion.bash in the repository. My consideration is that

  • copmletions/git.completion.sh would be a script to search and load the appropriate version of git-completion.bash
    1. If the completion for git is already defined, we do nothing.
    2. It first determines the git installation prefix based on the result of type -aP git. One might resolve the symbolic links when the obtained path by type -aP git is a symbolic link.
    3. It then tries to find $prefix/share/bash-completion/completions/git or $prefix/share/git-core/contrib/completion/git-completion.bash.
    4. Repeat 2--3 for each line of type -aP git and pick up the completion file found first. If nothing is found, try 3 with prefix=/usr. In the case that even that fails, finally we use git-completion.bash bundled with oh-my-bash as a fallback.
  • A fallback version would be included in tools/git-completion.bash in oh-my-bash.
  • Optionally, maybe we can also check whether the user prepares ~/.git-completion.bash

What do you think of this? If you agree with it, it would be nice if you could give it a try to implement it by yourself! I will finally review and adjust it if necessary, so please don't worry about the details but feel free to implement it. Questions are always welcome. But it is completely fine if you don't have enough time to do it by yourself. In that case, I can implement it.


#207 doesn't seem to be solved just by updating git-completion.bash because git-completion.bash doesn't know about the aliases defined by oh-my-bash. It seems like the completions for aliases are supposed to be set up by the users according to this section of git-completion.bash. We may add these settings in the place where we define git aliases. Or, we can actually turn on shopt -s progcomp_alias for Bash 5.0+ as I have made a comment in #156.


wget
https://raw.githubusercontent.com/git/git/v2.34.1/contrib/completion/git-completion.bash
--output-document=completions/git.completion.sh

As suggested in #102, I think we may include a script to update the bundled git-completion.bash. I don't think we need to update the file so frequently, but I rather think we can just prepare a script file and don't need to make a mechanism to automatically run it in the background.

@akinomyoga akinomyoga mentioned this pull request Dec 29, 2021
42 tasks
@jeansebastienh
Copy link
Contributor Author

You proposition is good.

I will try to implement it in the coming days. I will just need some help with the POSIX compatibility :trollface:

@jeansebastienh jeansebastienh marked this pull request as draft December 29, 2021 13:14
@akinomyoga
Copy link
Contributor

akinomyoga commented Dec 29, 2021

OK! Nice to hear that! Thank you!

@akinomyoga
Copy link
Contributor

@jeansebastienh Sorry, maybe I have requested too much. If you would like some help, please feel free to ask it! Or, if you are busy, I can take over the changes.

@jeansebastienh
Copy link
Contributor Author

Sorry I missed this issue.
I will try to propose something today

@akinomyoga
Copy link
Contributor

Thank you for the reply! No worries. Also, the initial draft doesn't need to be perfect, but we can implement these points one by one while discussing.

@jeansebastienh
Copy link
Contributor Author

I just rebase and submit a first proposition.

Copy link
Contributor

@akinomyoga akinomyoga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! This is sufficient for the first draft. I have quickly checked the implementation. One thing that I would like to request is that we want to run them in a function so that we can make temporary variables (such as prefixlist, prefix, file, _file, is_completion_loaded) local to the function. The function name can be e.g. _omb_completion_git_initialize, _omb_completion_git_loader, etc. There are several other comments.

completions/git.completion.sh Outdated Show resolved Hide resolved
completions/git.completion.sh Outdated Show resolved Hide resolved
completions/git.completion.sh Outdated Show resolved Hide resolved
completions/git.completion.sh Outdated Show resolved Hide resolved
completions/git.completion.sh Outdated Show resolved Hide resolved
@jeansebastienh
Copy link
Contributor Author

Thanks for the feedback. I have updated the PR.

Copy link
Contributor

@akinomyoga akinomyoga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! I think this is close to the final form.

split type -aP git by newlines?

Currently, we split the output of type -aP git by spaces (which are set to IFS by default). Here, we actually want to split the output of type -aP git by newlines in case there are some directory names containing spaces. Also, we would like to suppress the pathname expansions caused by * or ? possibly contained in the directory names. There are actually many different ways to achieve that but, for example,

local git_paths
IFS=$'\n' read -r -d '' -a git_paths <<< "$(type -aP git)"
for xxxx in "${git_paths[@]}"; do
  yyyy
done

Remaining tweaks

I have checked my initial consideration. It seems there are mainly three points that has not been addressed yet.

One might resolve the symbolic links when the obtained path by type -aP git is a symbolic link.

This can be tricky because we would like to use readlink -f or realpath that are not necessarily available in all the systems. I think I can add a fallback implementation of readlink -f, so you can use readlink -f at the moment.

If nothing is found, try 3 with prefix=/usr. In the case that even that fails,

I think we can just append /usr/bin/git in the output of type -aP git.

I guess we don't have to support this. The user can directory source their own file.

completions/git.completion.sh Outdated Show resolved Hide resolved
completions/git.completion.sh Outdated Show resolved Hide resolved
completions/git.completion.sh Outdated Show resolved Hide resolved
completions/git.completion.sh Outdated Show resolved Hide resolved
@jeansebastienh
Copy link
Contributor Author

I didn't find example of type -aP returning multiple entries so it was difficult to test / implement.

Regarding

Optionally, maybe we can also check whether the user prepares ~/.git-completion.bash
I guess we don't have to support this. The user can directory source their own file.

I agree with this and also we can suppose that the __gitdir function will exists (except for a home made completion)

Finaly I think I covered all your comments

@akinomyoga
Copy link
Contributor

Thank you!

I didn't find example of type -aP returning multiple entries so it was difficult to test / implement.

That typically happens when the user installs their own version of git other than the one installed by the distribution. I particularly don't have multiple gits either, but for example, I'm using a patched version of GNU Screen installed in my home directory. For the multiple screens in my system, the current code seems to work correctly.

$ type -aP screen
/home/murase/bin/screen
/home/murase/local/bin/screen
/usr/bin/screen
/bin/screen
$ IFS=$'\n' read -r -d '' -a screen_paths <<< "$(type -aP screen)"; declare -p screen_paths
declare -a screen_paths=([0]="/home/murase/bin/screen" [1]="/home/murase/local/bin/screen" [2]="/usr/bin/screen" [3]="/bin/screen")

I'll rebase the commits from now on.

@akinomyoga akinomyoga marked this pull request as ready for review February 5, 2022 04:23
@akinomyoga
Copy link
Contributor

I have rebased and adjusted trivial stuff. I'll later merge it. Thank you for your contributions, sincere and prompt responses, and patience for many requests!

- Use brace expansions to generate prefixed paths
- Check if the completion file is non-empty
- Specify explicit exit value to "return" for trap handlers
- Clean up the initialization function after initialization
@akinomyoga akinomyoga merged commit 92b4c30 into ohmybash:master Feb 7, 2022
@akinomyoga
Copy link
Contributor

I have merged it. Thank you again for your contributions!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants