All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Rast <trast@student.ethz.ch>
To: Andrew Sayers <andrew-git@pileofstuff.org>
Cc: "Shawn O. Pearce" <spearce@spearce.org>, <git@vger.kernel.org>
Subject: Re: [PATCH] bash completion: Support "divergence from upstream" messages in __git_ps1
Date: Mon, 14 Jun 2010 09:42:42 +0200	[thread overview]
Message-ID: <201006140942.43099.trast@student.ethz.ch> (raw)
In-Reply-To: <4C13F32B.7060106@pileofstuff.org>

Andrew Sayers wrote:
> I've added a message in the "equal to upstream" case, to differentiate
> it from the "no upstream" case.  Again, this is an over-the-shoulder
> issue - when I see an "=" (or " u=") in someone's prompt, I don't have
> to patronise them about whether they've e.g. misconfigured their
> branch.

I omitted it because I thought it would be too cluttery, but then my
branches seem to rarely agree with their upstream.

> +       local cfg=( $( git config --get-regexp '^bash\.showUpstream$|^svn-remote\..*\.url$' 2>/dev/null ) )

Doesn't this break if the config value contains spaces?  I don't know
enough about bash arrays but in my simple tests, the array elements
are split between words.

And with the new design, you practically *expect* the config key to
contain spaces.

Along the same lines, I think
> +                               GIT_PS1_SHOWUPSTREAM="${cfg[$((n+1))]}"
> +                               if [[ -z "${GIT_PS1_SHOWUPSTREAM}" ]]; then
can never trigger because bash will never see the empty config string.

Slightly more robust would be to use

  git config --get-regexp '^bash\.showUpstream$|^svn-remote\..*\.url$' \
    2>/dev/null |
  while read key value, do
    # stuff
  done

That still breaks in the case of values containing newlines, though.

> I like the "ref" option, but I'm not really sure when "eval" would be
> useful.  I've changed it here to "cmd" so people are encouraged to put
> their work in a script.
[...]
> +#           cmd=<command> compare HEAD to the output of <command>
[...]
> +		cmd\=*) upstream=$( "${option:4}" ) ;;

"Encourage" is a mild understatement; AFAICS the code doesn't work
with more than single-word command any more.

The original intent was that the user could put a (very small) shell
script directly in the configuration if the normal DWIMming doesn't
fit his neds, perhaps most likely in the case of git-svn (do other
remote helpers have the same problem?).

Having to wrap it in a script defeats that point, as it becomes almost
as easy to edit the completion script.  So I think if it can't eval,
you might as well remove it.

BTW, please spell $(command) substitution without the spaces.  Your
current style does not match what is already in the file.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

  reply	other threads:[~2010-06-14  7:42 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-06-06  0:05 [PATCH] bash completion: Support "unpushed commits" warnings in __git_ps1 Andrew Sayers
2010-06-06 18:14 ` Thomas Rast
2010-06-06 20:49   ` Andrew Sayers
2010-06-06 21:07     ` Jakub Narebski
2010-06-06 22:19       ` Andrew Sayers
2010-06-07  7:42     ` Thomas Rast
2010-06-08 21:36       ` [RFC/PATCHv2] bash completion: Support "divergence from upstream" " Andrew Sayers
2010-06-09  8:21         ` Peter Kjellerstedt
2010-06-09  8:45         ` John Tapsell
2010-06-09 21:02           ` Steven Michalske
2010-06-09  9:17         ` Michael J Gruber
2010-06-09 20:48           ` Michael J Gruber
2010-06-09 21:03             ` Michael J Gruber
2010-06-10 11:47         ` [PATCH 0/2] " Thomas Rast
2010-06-10 11:47           ` [PATCH 1/2] rev-list: introduce --count option Thomas Rast
2010-06-10 11:47           ` [PATCH 2/2] bash completion: Support "divergence from upstream" warnings in __git_ps1 Thomas Rast
2010-06-12  0:00             ` SZEDER Gábor
2010-06-12 10:03               ` [PATCH v2 0/2] " Thomas Rast
2010-06-12  9:59                 ` [PATCH v2 1/2] rev-list: introduce --count option Thomas Rast
2010-06-12  9:59                 ` [PATCH v2 2/2] bash completion: Support "divergence from upstream" warnings in __git_ps1 Thomas Rast
2010-06-14  3:13                   ` Junio C Hamano
2010-06-14  7:44                     ` Thomas Rast
2010-06-14 12:36                   ` SZEDER Gábor
2010-06-12 10:11                 ` vger doesn't like UTF-8 from send-email Thomas Rast
2010-06-12 15:06                   ` [PATCH] send-email: ask about and declare 8bit mails Thomas Rast
2010-06-12 16:28                     ` Junio C Hamano
2010-06-13 15:09                       ` Thomas Rast
2010-06-13  4:15                   ` vger doesn't like UTF-8 from send-email Michael Witten
2010-06-14 11:57                     ` Erik Faye-Lund
2010-06-12 20:50                 ` [PATCH] bash completion: Support "divergence from upstream" messages in __git_ps1 Andrew Sayers
2010-06-14  7:42                   ` Thomas Rast [this message]
2010-06-15 21:50                     ` [PATCHv4] " Andrew Sayers
2010-06-16 19:05                       ` Junio C Hamano
2010-06-16 19:11                         ` Thomas Rast
2010-06-17 21:31                         ` [PATCHv5 0/2] " Andrew Sayers
2010-06-18 16:10                           ` Junio C Hamano
2010-06-18 21:02                             ` Andrew Sayers
2010-06-17 21:32                         ` [PATCHv5 1/2] " Andrew Sayers
2010-06-17 21:32                         ` [PATCHv5 2/2] bash-completion: Fix __git_ps1 to work with "set -u" Andrew Sayers
2010-06-10 13:31           ` [PATCH 0/2] bash completion: Support "divergence from upstream" warnings in __git_ps1 Michael J Gruber
2010-06-10 12:03         ` [RFC/PATCHv2] " Thomas Rast
2010-06-06 20:12 ` [PATCH] bash completion: Support "unpushed commits" " Thomas Rast
  -- strict thread matches above, loose matches on Subject: below --
2010-06-23 22:09 What's cooking in git.git (Jun 2010, #04; Wed, 23) Junio C Hamano
     [not found] ` <7veifxe63j.fsf@alter.siamese.dyndns.org>
2010-06-23 22:54   ` [PATCH] bash completion: Support "divergence from upstream" messages in __git_ps1 Shawn O. Pearce

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=201006140942.43099.trast@student.ethz.ch \
    --to=trast@student.ethz.ch \
    --cc=andrew-git@pileofstuff.org \
    --cc=git@vger.kernel.org \
    --cc=spearce@spearce.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.