Re: [PATCH 1/4] diff-highlight: add `less -r` to cmd in README

2015-11-03 Thread Jeff King
On Tue, Nov 03, 2015 at 01:51:54PM -0800, Junio C Hamano wrote:

> Jeff King  writes:
> 
> > I agree with Junio that "-R" is a more sensible default, but I don't
> > think that should be necessary. We already set LESS when running the
> > pager (and if you are overriding it, you are already in trouble, because
> > git itself will generate ANSI codes by default).
> 
> I do not quite understand this part.  Inside git itself when we
> spawn the pager we export LESS with a sensible default, exactly to
> help users who do not have LESS set and exported.  This one however
> is not spawned by git but the end-user piping output of diff-hilite.

Oh, you're right. I mistook it as instructions for what to put in
pager.log, but that is clearly not what this is. I agree that telling
the user to use "-R" here is a good idea.

And indeed, the instructions just below the hunk in question describe
setting it properly in your config (with less, and no "-R"). So I was
simply confused and not reading carefully enough.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/4] diff-highlight: add `less -r` to cmd in README

2015-11-03 Thread Jeff King
On Mon, Nov 02, 2015 at 09:05:31PM -0500, Jonathan Lebon wrote:

> As it is, the suggested command for trying out diff-highlight will just
> dump the whole git log output to the terminal. Let's pipe it through
> `less` so users aren't surprised on the first try.

That makes sense. I use "diff-highlight | less" myself, of course. I
think I excluded it from the README in the name of simplicity, but you
are right that it is probably better to give a complete working example.

I agree with Junio that "-R" is a more sensible default, but I don't
think that should be necessary. We already set LESS when running the
pager (and if you are overriding it, you are already in trouble, because
git itself will generate ANSI codes by default).

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/4] diff-highlight: add `less -r` to cmd in README

2015-11-03 Thread Junio C Hamano
Jeff King  writes:

> I agree with Junio that "-R" is a more sensible default, but I don't
> think that should be necessary. We already set LESS when running the
> pager (and if you are overriding it, you are already in trouble, because
> git itself will generate ANSI codes by default).

I do not quite understand this part.  Inside git itself when we
spawn the pager we export LESS with a sensible default, exactly to
help users who do not have LESS set and exported.  This one however
is not spawned by git but the end-user piping output of diff-hilite.

I agree that if the user has LESS exported without -R that would not
be pretty while viewing coloured output.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/4] diff-highlight: add `less -r` to cmd in README

2015-11-02 Thread Junio C Hamano
Jonathan Lebon  writes:

> As it is, the suggested command for trying out diff-highlight will just
> dump the whole git log output to the terminal. Let's pipe it through
> `less` so users aren't surprised on the first try.

That justifies the "less" part but not your choice of "-r".

I am assuming that you are telling "less" not to show the ANSI
"color" escape sequences using the caret notation with "-r", which
is a very natural and sensible thing to do when using `highlight`.

But if that is the case, you don't want "-r" (raw control chars for
everything).  You would want to say "-R", I think.

Other than that, looks like a sensible thing to do to me.

Thanks.


> Signed-off-by: Jonathan Lebon 
> ---
>  contrib/diff-highlight/README | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/contrib/diff-highlight/README b/contrib/diff-highlight/README
> index 836b97a..bbbfdda 100644
> --- a/contrib/diff-highlight/README
> +++ b/contrib/diff-highlight/README
> @@ -44,9 +44,9 @@ Use
>  
>  You can try out the diff-highlight program with:
>  
> --
> -git log -p --color | /path/to/diff-highlight
> --
> +--
> +git log -p --color | /path/to/diff-highlight | less -r
> +--
>  
>  If you want to use it all the time, drop it in your $PATH and put the
>  following in your git configuration:
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/4] diff-highlight: add `less -r` to cmd in README

2015-11-02 Thread Jonathan Lebon
On Mon, Nov 2, 2015 at 9:41 PM, Junio C Hamano  wrote:
>
> Jonathan Lebon  writes:
>
> > As it is, the suggested command for trying out diff-highlight will just
> > dump the whole git log output to the terminal. Let's pipe it through
> > `less` so users aren't surprised on the first try.
>
> That justifies the "less" part but not your choice of "-r".
>
> I am assuming that you are telling "less" not to show the ANSI
> "color" escape sequences using the caret notation with "-r", which
> is a very natural and sensible thing to do when using `highlight`.
>
> But if that is the case, you don't want "-r" (raw control chars for
> everything).  You would want to say "-R", I think.

Ahh thanks, that makes sense. I will update this for v2 tomorrow.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/4] diff-highlight: add `less -r` to cmd in README

2015-11-02 Thread Jonathan Lebon
As it is, the suggested command for trying out diff-highlight will just
dump the whole git log output to the terminal. Let's pipe it through
`less` so users aren't surprised on the first try.

Signed-off-by: Jonathan Lebon 
---
 contrib/diff-highlight/README | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/contrib/diff-highlight/README b/contrib/diff-highlight/README
index 836b97a..bbbfdda 100644
--- a/contrib/diff-highlight/README
+++ b/contrib/diff-highlight/README
@@ -44,9 +44,9 @@ Use
 
 You can try out the diff-highlight program with:
 
--
-git log -p --color | /path/to/diff-highlight
--
+--
+git log -p --color | /path/to/diff-highlight | less -r
+--
 
 If you want to use it all the time, drop it in your $PATH and put the
 following in your git configuration:
-- 
2.6.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html