@jlesquembre, thanks for your PR! By analyzing the history of the files in this pull request, we identified @benley, @MarcWeber and @abbradar to be potential reviewers.

How does git-open capture the in-store git binary?

@joachifm Thanks for your feedback, I updated the PR to remove the .... About your question, git-open doesn't really capture the git binary. In short, when you execute git foo, git looks for git-foo in your PATH and calls it. A better explanation from the official git documentation:

Git subcommands are standalone executables that live in the Git exec path, normally /usr/lib/git-core. The git executable itself is a thin wrapper that knows where the subcommands live, and runs them by passing command-line arguments to them. (If "git foo" is not found in the Git exec path, the wrapper will look in the rest of your $PATH for it. Thus, it’s possible to write local Git extensions that don’t live in system space.)

For even more information:

• How to integrate new subcommands
• git builtin API

PD: I also updated git-and-tools/default.nix to keep git-open in alphabetical order

What I mean is, git-open assumes git is in PATH, which may or may not hold (i.e., it is a runtime impurity). I expected the reason you added git to build inputs was to ensure its availability at runtime.
For optional features i think it's okay to fail at runtime if it's clear to the user how to resolve it, but if it's a basic requirement for the program to work at all it should be patched in/wrapped.

@Mic92 sure, it's a command I use regularly, I'll add myself as maintainer

@joachifm I think now I understand what you mean. You are right, I added git to ensure its availability, but the correct way to do it would be:

  installPhase = ''
    mkdir -p $out/bin
    cp git-open $out/bin
    wrapProgram $out/bin/git-open \
      --prefix PATH ':' "${git}/bin"
  '';

Right? I'm new to NixOS, so if you see room for improvement, corrections are welcome.

yes, the fix is correct.

@jlesquembre LGTM. Since this is a bash script, you could patch the source directly to avoid the indirection, but your solution is pretty standard.

I notice that the script also has implicit dependencies on xdg-open and (e)grep. The latter can be safely assumed, but xdg-open may need to be added as well. You can use the makeBinPath helper (from stdenv.lib) for convenience.

@joachifm Thanks again for the feedback, today I learned a lot about nix packages.

I added xdg_tools and gnugrep as dependencies, I hope that I didn't miss anything.

Thanks!