All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Simon Oosthoek <soosthoek@nieuwland.nl>
Cc: git@vger.kernel.org, s.oosthoek@xs4all.nl,
	Michael J Gruber <git@drmicha.warpmail.net>,
	"Shawn O. Pearce" <spearce@spearce.org>,
	Ramkumar Ramachandra <artagnon@gmail.com>,
	schwab@linux-m68k.org
Subject: Re: [PATCH] Add __git_ps1_pc to use as PROMPT_COMMAND
Date: Mon, 01 Oct 2012 10:16:49 -0700	[thread overview]
Message-ID: <7v626udse6.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <50695ECE.5010101@nieuwland.nl> (Simon Oosthoek's message of "Mon, 01 Oct 2012 11:13:50 +0200")

Simon Oosthoek <soosthoek@nieuwland.nl> writes:

> On 09/28/2012 07:58 PM, Junio C Hamano wrote:
>> Simon Oosthoek <soosthoek@nieuwland.nl> writes:
>>
>>> +# __git_ps1_pc accepts 0 arguments (for now)
>>> +# It is meant to be used as PROMPT_COMMAND, it sets PS1
>>> +__git_ps1_pc ()
>>> +{
>>> +	local g="$(__gitdir)"
>>> +	if [ -n "$g" ]; then
>>> +...
>>> +	fi
>>> +}
>>
>> This looks awfully similar to the existing code in __git_ps1
>> function.  Without refactoring to share the logic between them, it
>> won't be maintainable.
>>
>
> I agree that it's ugly. How about the following:
>
> I modified __git_ps1 to work both in PROMPT_COMMAND mode and in that
> mode support color hints.
>
> This way there's one function, so no overlap.

I think the logical progression would be

 - there are parts of __git_ps1 you want to reuse for your
   __git_ps1_pc; separate that part out as a helper function,
   and make __git_ps1 call it, without changing what __git_ps1
   does (i.e. no colors, etc.)

 - add __git_ps1_pc that uses the helper function you separated
   out.

 - add whatever bells and whistles that are useful for users of
   either __git_ps1 or __git_ps1_pc to that helper function.

  reply	other threads:[~2012-10-01 17:17 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-09-26 15:00 bash completion with colour hints Simon Oosthoek
2012-09-26 15:24 ` Ramkumar Ramachandra
2012-09-26 19:25   ` Simon Oosthoek
2012-09-27  6:53     ` Junio C Hamano
2012-09-27  8:53       ` Michael J Gruber
2012-09-27  8:55         ` Michael J Gruber
2012-09-27  9:16         ` Simon Oosthoek
2012-09-27 10:05         ` Andreas Schwab
2012-09-27 11:57         ` Simon Oosthoek
2012-09-28 11:40         ` [PATCH] Add __git_ps1_pc to use as PROMPT_COMMAND Simon Oosthoek
2012-09-28 17:58           ` Junio C Hamano
2012-10-01  9:13             ` Simon Oosthoek
2012-10-01 17:16               ` Junio C Hamano [this message]
2012-10-01 18:42                 ` Simon Oosthoek
2012-10-01 19:13                 ` Junio C Hamano
2012-10-01 19:27                   ` Simon Oosthoek
2012-10-01 19:54                     ` Junio C Hamano
2012-10-01 20:56                       ` Simon Oosthoek
2012-10-01 21:09                         ` Junio C Hamano
2012-10-02  7:38                           ` Michael J Gruber
2012-10-02  8:01                             ` Simon Oosthoek
2012-10-02 17:01                             ` Junio C Hamano
2012-10-02 19:50                               ` Simon Oosthoek
2012-10-02 20:18                                 ` Junio C Hamano
2012-10-05 21:09                                   ` [PATCH 1/2] Allow __git_ps1 to be used in PROMPT_COMMAND Simon Oosthoek
2012-10-08 18:12                                     ` Junio C Hamano
2012-10-08 19:50                                       ` Simon Oosthoek
2012-10-08 21:17                                         ` Junio C Hamano
2012-10-10 19:31                                           ` Simon Oosthoek
2012-10-10 23:00                                             ` Junio C Hamano
2012-10-10 19:32                                           ` [PATCH 2/2] show color hints based on state of the git tree Simon Oosthoek
2012-10-10 19:37                                           ` [PATCH 1/2] Allow __git_ps1 to be used in PROMPT_COMMAND Simon Oosthoek
2012-10-10 19:38                                           ` [PATCH 2/2] show color hints based on state of the git tree Simon Oosthoek
2012-10-05 21:10                                   ` Simon Oosthoek
2012-10-15  8:23                                     ` Michael J Gruber
2012-10-15  9:01                                       ` Simon Oosthoek
2012-10-15  9:13                                         ` Michael J Gruber
2012-10-15 10:34                                           ` Simon Oosthoek
2012-10-15 13:20                                           ` Simon Oosthoek
2012-10-15 15:19                                             ` Michael J Gruber
     [not found]                                           ` <CAPc5daVUyAuznmrT+-yqvPR0gd38oiWmi2k+BFVV1s9ouMUt0Q@mail.gmail.com>
2012-10-15 15:15                                             ` Simon Oosthoek
2012-10-15 18:10                                               ` Junio C Hamano
2012-10-16  5:32                                                 ` [PATCH 3/3] Change colors to be based on git status -sb in color mode Simon Oosthoek
2012-10-16 15:58                                                   ` Junio C Hamano
2012-10-16 19:34                                                     ` Simon Oosthoek
2012-10-16 21:30                                                       ` Junio C Hamano
2012-10-16 22:04                                                         ` Junio C Hamano
2012-10-17  7:17                                                         ` Simon Oosthoek
2012-10-08 15:00                                   ` [PATCH] Add __git_ps1_pc to use as PROMPT_COMMAND Simon Oosthoek

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=7v626udse6.fsf@alter.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=artagnon@gmail.com \
    --cc=git@drmicha.warpmail.net \
    --cc=git@vger.kernel.org \
    --cc=s.oosthoek@xs4all.nl \
    --cc=schwab@linux-m68k.org \
    --cc=soosthoek@nieuwland.nl \
    --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.