Jeff KingThe new 'git-browse-help' seems to purely be a helper script for "git help" (judging from the commit messages and the lack of documentation). Should it perhaps be "git--browse-help" or "git-help--browse" to follow our "this is not for users" convention (see add--interactive, fetch--tool, and rebase--interactive).
I became aware of it because "br" used to tab-complete "branch" but now doesn't. :) And if we're going to change it, pre-1.5.4 is the time.
-Peff
Junio C HamanoRe: git-browse-help?
Jeff King <peff@peff.net> writes:
> The new 'git-browse-help' seems to purely be a helper script for "git > help" (judging from the commit messages and the lack of documentation). > Should it perhaps be "git--browse-help" or "git-help--browse" to follow > our "this is not for users" convention (see add--interactive, > fetch--tool, and rebase--interactive). > > I became aware of it because "br" used to tab-complete "branch" but now > doesn't. :) And if we're going to change it, pre-1.5.4 is the time.
Good eyes and a sensible argument. Go wild.
Jeff KingRe: git-browse-help?
On Fri, Dec 14, 2007 at 09:38:05AM -0800, Junio C Hamano wrote:
> > Should it perhaps be "git--browse-help" or "git-help--browse" to follow > Good eyes and a sensible argument. Go wild.
Looks like you beat me to it. Thanks.
-Peff
Jeff KingRe: git-browse-help?
On Sat, Dec 15, 2007 at 05:08:11AM -0500, Jeff King wrote:
> On Fri, Dec 14, 2007 at 09:38:05AM -0800, Junio C Hamano wrote: > > > > Should it perhaps be "git--browse-help" or "git-help--browse" to follow > > Good eyes and a sensible argument. Go wild. > > Looks like you beat me to it. Thanks.
Although I would have called it "git-help--browse" rather than "git-browse--help" since
a) it is related to "git help", not "git browse"
b) there is no tab completion conflict with git-he, but there is with
git-br. :) This is of course because I am using the file
completion, but the programmable bash completion has the same
problem. Shawn, perhaps it should simply ignore *--* as plumbing?I can prepare a patch if "git-help--browse" makes more sense.
-Peff
Jeff KingRe: git-browse-help?
On Sat, Dec 15, 2007 at 06:01:53AM -0500, Jeff King wrote:
> problem. Shawn, perhaps it should simply ignore *--* as plumbing?
And here is a patch to do that.
-- >8 -- teach bash completion to treat commands with "--" as plumbing
There is a convention that commands containing a double-dash are implementation details and not to be used by mortals. We should automatically remove them from the completion suggestions as plumbing.
Signed-off-by: Jeff King <peff@peff.net> --- BTW, Junio, I remember discussion a long time ago about doing:
cover letter -- >8 -- commit --- diff
versus
commit --- cover letter diff
and I recall that you did not have a strong preference. I have started using the former, as I find it a bit more convenient to write (and I think it is more readable when you are following up a discussion rather than writing a real cover letter or commenting on the patch). However, I wanted to confirm that you don't find it significantly more annoying.
contrib/completion/git-completion.bash | 3 +-- 1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 58e0e53..2fd32db 100755 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -291,7 +291,7 @@ __git_commands () for i in $(git help -a|egrep '^ ') do case $i in - add--interactive) : plumbing;; + *--*) : plumbing pattern;; applymbox) : ask gittus;; applypatch) : ask gittus;; archimport) : import;; @@ -308,7 +308,6 @@ __git_commands () diff-tree) : plumbing;; fast-import) : import;; fsck-objects) : plumbing;; - fetch--tool) : plumbing;; fetch-pack) : plumbing;; fmt-merge-msg) : plumbing;; for-each-ref) : plumbing;;
-- 1.5.4.rc0.1124.g1b66f-dirty
Junio C HamanoRe: git-browse-help?
Jeff King <peff@peff.net> writes:
> BTW, Junio, I remember discussion a long time ago about doing: > > cover letter > -- >8 -- > commit > --- > diff > > versus > > commit > --- > cover letter > diff > > and I recall that you did not have a strong preference. I have started > using the former, as I find it a bit more convenient to write (and I > think it is more readable when you are following up a discussion rather > than writing a real cover letter or commenting on the patch).
Yeah, and it is not too inconvenient to trim it off if you use "am -i" or "commit --amend". An added bonus is unlike the "top-post" style, the cover material is available when editing the final log message, so I actually slightly prefer "cover -- >8 -- log --- patch" myself.
Shawn O. PearceRe: git-browse-help?
Jeff King <peff@peff.net> wrote:
> -- >8 -- > teach bash completion to treat commands with "--" as plumbing > > There is a convention that commands containing a double-dash > are implementation details and not to be used by mortals. We > should automatically remove them from the completion > suggestions as plumbing. > > Signed-off-by: Jeff King <peff@peff.net>
Acked-by: Shawn O. Pearce <spearce@spearce.org>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash > index 58e0e53..2fd32db 100755 > --- a/contrib/completion/git-completion.bash > +++ b/contrib/completion/git-completion.bash > @@ -291,7 +291,7 @@ __git_commands () > for i in $(git help -a|egrep '^ ') > do > case $i in > - add--interactive) : plumbing;; > + *--*) : plumbing pattern;; > applymbox) : ask gittus;; > applypatch) : ask gittus;; > archimport) : import;; > @@ -308,7 +308,6 @@ __git_commands () > diff-tree) : plumbing;; > fast-import) : import;; > fsck-objects) : plumbing;; > - fetch--tool) : plumbing;; > fetch-pack) : plumbing;; > fmt-merge-msg) : plumbing;; > for-each-ref) : plumbing;;
-- Shawn.
Junio C HamanoRe: git-browse-help?
Jeff King <peff@peff.net> writes:
> Although I would have called it "git-help--browse" rather than > "git-browse--help" since
Good point.
Jeff KingRe: git-browse-help?
On Sat, Dec 15, 2007 at 11:26:44AM -0800, Junio C Hamano wrote:
> > Although I would have called it "git-help--browse" rather than > > "git-browse--help" since > > Good point.
And here is the patch (I based the change-sites off of the ones in your previous patch, plus a grep to double-check).
-- >8 -- rename git-browse--help to git-help--browse
The convention for helper scripts has been git-$TOOL--$HELPER. Since this is a "browse" helper for the "help" tool, git-help--browse is a more sensible name.
Signed-off-by: Jeff King <peff@peff.net> --- .gitignore | 2 +- Documentation/git-help.txt | 4 ++-- Makefile | 2 +- git-browse--help.sh => git-help--browse.sh | 0 help.c | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) rename git-browse--help.sh => git-help--browse.sh (100%)
diff --git a/.gitignore b/.gitignore index aef01c5..dab5bc2 100644 --- a/.gitignore +++ b/.gitignore @@ -12,7 +12,6 @@ git-archive git-bisect git-blame git-branch -git-browse--help git-bundle git-cat-file git-check-attr @@ -52,6 +51,7 @@ git-gc git-get-tar-commit-id git-grep git-hash-object +git-help--browse git-http-fetch git-http-push git-imap-send diff --git a/Documentation/git-help.txt b/Documentation/git-help.txt index 8cd69e7..da3f718 100644 --- a/Documentation/git-help.txt +++ b/Documentation/git-help.txt @@ -47,13 +47,13 @@ OPTIONS + The web browser can be specified using the configuration variable 'help.browser', or 'web.browser' if the former is not set. If none of -these config variables is set, the 'git-browse--help' helper script +these config variables is set, the 'git-help--browse' helper script (called by 'git-help') will pick a suitable default. + You can explicitly provide a full path to your prefered browser by setting the configuration variable 'browser.<tool>.path'. For example, you can configure the absolute path to firefox by setting -'browser.firefox.path'. Otherwise, 'git-browse--help' assumes the tool +'browser.firefox.path'. Otherwise, 'git-help--browse' assumes the tool is available in PATH. + Note that the script tries, as much as possible, to display the HTML diff --git a/Makefile b/Makefile index 62f1893..7776077 100644 --- a/Makefile +++ b/Makefile @@ -227,7 +227,7 @@ SCRIPT_SH = \ git-lost-found.sh git-quiltimport.sh git-submodule.sh \ git-filter-branch.sh \ git-stash.sh \ - git-browse--help.sh + git-help--browse.sh SCRIPT_PERL = \ git-add--interactive.perl \ diff --git a/git-browse--help.sh b/git-help--browse.sh similarity index 100% rename from git-browse--help.sh rename to git-help--browse.sh diff --git a/help.c b/help.c index f9ce6db..1302a61 100644 --- a/help.c +++ b/help.c @@ -331,7 +331,7 @@ static void show_info_page(const char *git_cmd) static void show_html_page(const char *git_cmd) { const char *page = cmd_to_page(git_cmd); - execl_git_cmd("browse--help", page, NULL); + execl_git_cmd("help--browse", page, NULL); } void help_unknown_cmd(const char *cmd)
-- 1.5.4.rc0.1122.g899d-dirty