Re: Bug: pager. doesn't work well with editors

> now, because it is handled outside of the "tag" command entirely, and > > doesn't know what mode the tag command will be running in. > > Stepping back even further, perhaps the whole pager. was a bad > interim move. For those who set "less" without "-F&

Re: Bug: pager. doesn't work well with editors

doesn't know what mode the tag command will be running in. Stepping back even further, perhaps the whole pager. was a bad interim move. For those who set "less" without "-F", being able to set pager. to false may still be necessary, but I am wondering about setting it to true or a

Re: Bug: pager. doesn't work well with editors

On Mon, Sep 19, 2016 at 09:03:05AM -0700, Junio C Hamano wrote: > Anatoly Borodin <anatoly.boro...@gmail.com> writes: > > >> I think, the pagination should be turned off when the editor is being > >> called. > > This is a fun one. IIRC, we decide to spawn a

Re: Bug: pager. doesn't work well with editors

Anatoly Borodin <anatoly.boro...@gmail.com> writes: >> I think, the pagination should be turned off when the editor is being >> called. This is a fun one. IIRC, we decide to spawn a pager and run our output via pipe thru it fairly early, even before we figure out which subco

Re: Bug: pager. doesn't work well with editors

> I think, the pagination should be turned off when the editor is being > called. ... even if the `[-p|--paginate]` option is used explicitly. Is there a case when pagination shouldn't be ignored with an editor? `git -p -c core.editor=gvim config -e` works, but the pagination is not effective.

Bug: pager. doesn't work well with editors

Hi All! It can be useful to enable `pager.branch`, `pager.tag`, `pager.config`, etc for some projects (`git` itself can be a good example, with all its feature branches and tags). But it makes commands like `git branch --edit-description`, `git tag -a`, `git config -e` extremely unhappy. For

[PATCH 11/16] pager: handle early config

The pager code is often run early in the git.c startup, before we have actually found the repository. When we ask git_config() to look for values like core.pager, it doesn't know where to find the repo-level config, and will blindly examine ".git/config" if it exists. That's why t

[PATCH 10/16] pager: use callbacks instead of configset

While the cached configset interface is more pleasant to use, it is not appropriate for "early" config like pager setup, which must sometimes do tricky things like reading from ".git/config" even when we have not set up the repository. As a preparatory step to handling these

[PATCH 09/16] pager: make pager_program a file-local static

This variable is only ever used by the routines in pager.c, and other parts of the code should always use those routines (like git_pager()) to make decisions about which pager to use. Let's reduce its scope to prevent accidents. Signed-off-by: Jeff King <p...@peff.net> --- cache.h

[PATCH 08/16] pager: stop loading git_default_config()

a bad idea here, for two reasons: 1. The pager setup may be called very early in the program, before we have found the git repository. As a result, we may fail to read the correct repo-level config file. This is a problem for core.pager, too, but we should at least try

[PATCH 07/16] pager: remove obsolete comment

#ifdefs into this file anyway. And then even later, in ea27a18 (spawn pager via run_command interface, 2008-07-22) we unified the implementations. So these days this comment is really saying nothing at all. Let's drop it. Signed-off-by: Jeff King <p...@peff.net> --- pager.c | 5 -

Re: [PATCH v3] pager: move pager-specific setup into the build

On Thu, Aug 04, 2016 at 11:34:10AM +, Eric Wong wrote: > > > --- a/config.mak.uname > > > +++ b/config.mak.uname > > > @@ -209,6 +209,7 @@ ifeq ($(uname_S),FreeBSD) > > > HAVE_PATHS_H = YesPlease > > > GMTIME_UNRELIABLE_ERRORS = UnfortunatelyYes > > > HAVE_BSD_SYSCTL = YesPlease > > > +

[PATCH v4] pager: move pager-specific setup into the build

Allowing PAGER_ENV to be set at build-time allows us to move pager-specific knowledge out of our build. This allows us to set a better default for FreeBSD more(1), which pretends not to understand ANSI color escapes if the MORE environment variable is left empty, but accepts the same variables

Re: [PATCH v3] pager: move pager-specific setup into the build

= $(subst ','\'',$(PAGER_ENV_CQ)) BASIC_CFLAGS += -DPAGER_ENV='$(PAGER_ENV_CQ_SQ)' diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh index e4fc5c8..c8dc665 100755 --- a/t/t7006-pager.sh +++ b/t/t7006-pager.sh @@ -49,6 +49,19 @@ test_expect_success TTY 'LESS and L

Re: [PATCH v3] pager: move pager-specific setup into the build

is one spot, not invading the whole Makefile). Feel free to ignore it as over-engineered, but I thought I'd throw it out there in case it appeals. -- >8 -- diff --git a/.gitignore b/.gitignore index 05cb58a..b0fd5ec 100644 --- a/.gitignore +++ b/.gitignore @@ -180,6 +180,7 @@ /gitweb/static/gi

[PATCH v3] pager: move pager-specific setup into the build

From: Junio C Hamano <gits...@pobox.com> Allowing PAGER_ENV to be set at build-time allows us to move pager-specific knowledge out of our build. This allows us to set a better default for FreeBSD more(1), which misbehaves if MORE environment variable is left empty, but accepts the same var

Re: [PATCH 1/1 v2] pager: move pager-specific setup into the build

On Wed, Aug 03, 2016 at 01:57:09PM -0700, Junio C Hamano wrote: > All bugs are from my original, I think. Here is a proposed squash. > > * This shouldn't make much difference for @@PAGER_ENV@@, but not >quoting the default assignment, i.e. > > : "${VAR=VAL}" && export VAR > >is

Re: [PATCH 1/1 v2] pager: move pager-specific setup into the build

Eric Wong writes: > Junio C Hamano wrote: >> All bugs are from my original, I think. Here is a proposed squash. > > Thanks, I'll take the git-sh-setup changes. > > I actually just rewrote setup_pager_env using split_cmdline and > eliminated all the scary (to

Re: [PATCH 1/1 v2] pager: move pager-specific setup into the build

Junio C Hamano wrote: > All bugs are from my original, I think. Here is a proposed squash. Thanks, I'll take the git-sh-setup changes. I actually just rewrote setup_pager_env using split_cmdline and eliminated all the scary (to me) pointer arithmetic and avoided strbuf, too.

Re: [PATCH 1/1 v2] pager: move pager-specific setup into the build

.*s", +(int)(cp - pager_env), pager_env); + pager_env = cp + strspn(cp, " "); strbuf_reset(); - while (*cp && *cp == ' ') - cp++; - pager_env = cp; }

Re: [PATCH 1/1 v2] pager: move pager-specific setup into the build

On Mon, Aug 01, 2016 at 09:49:37PM +, Eric Wong wrote: > +static void setup_pager_env(struct argv_array *env) > +{ > + const char *pager_env = PAGER_ENV; > + > + while (*pager_env) { > + struct strbuf buf = STRBUF_INIT; > + const char *cp = strchrnul(pager_env,

Re: [PATCH 1/1 v2] pager: move pager-specific setup into the build

Eric Wong writes: > So v3 will be MORE=FRX, as less was added: > > commit 98170c0c3ba86eb1cc975e7848d075bf2abc1ed0 > Author: ps > Date: Mon May 22 10:00:00 2000 + > > bmake glue for less. > > and more was nuked: > > commit

Re: [PATCH 1/1 v2] pager: move pager-specific setup into the build

Jeff King <p...@peff.net> writes: > On Mon, Aug 01, 2016 at 04:03:40PM -0700, Junio C Hamano wrote: > >> Eric Wong <e...@80x24.org> writes: >> >> > From: Junio C Hamano <gits...@pobox.com> >> > >> > Allowing PAGER_ENV to be set at

Re: [PATCH 1/1 v2] pager: move pager-specific setup into the build

Eric Wong <e...@80x24.org> writes: > From: Junio C Hamano <gits...@pobox.com> > > Allowing PAGER_ENV to be set at build-time allows us to move > pager-specific knowledge out of our build. Currently, this > allows us to set a better default for FreeBSD where more(1) >

Re: [PATCH 1/1 v2] pager: move pager-specific setup into the build

Junio C Hamano <gits...@pobox.com> wrote: > Eric Wong <e...@80x24.org> writes: > > Allowing PAGER_ENV to be set at build-time allows us to move > > pager-specific knowledge out of our build. Currently, this > > allows us to set a better default for FreeBSD whe

Re: [PATCH 1/1 v2] pager: move pager-specific setup into the build

On Mon, Aug 01, 2016 at 04:03:40PM -0700, Junio C Hamano wrote: > Eric Wong <e...@80x24.org> writes: > > > From: Junio C Hamano <gits...@pobox.com> > > > > Allowing PAGER_ENV to be set at build-time allows us to move > > pager-specific knowledge out of

[PATCH 1/1 v2] pager: move pager-specific setup into the build

From: Junio C Hamano <gits...@pobox.com> Allowing PAGER_ENV to be set at build-time allows us to move pager-specific knowledge out of our build. Currently, this allows us to set a better default for FreeBSD where more(1) is the same binary as less(1). This also prepares us for introducing

Re: [PATCH 1/2] pager: move pager-specific setup into the build

>> > Allowing PAGER_ENV to be set at build-time allows us to move >> > pager-specific knowledge out of our build. Currently, this >> > allows us to set a better default for FreeBSD where more(1) >> > is the same binary as less(1). >> >> Nice. I was

Re: [PATCH 1/2] pager: move pager-specific setup into the build

Jeff King writes: > I'm not too worried about spaces here. This is a resurrection of an old > discussion, and in all that time, I think the only realistic suggestions > for built-in values have been pretty tame. > > If this were used to parse arbitrary user-provided runtime

Re: [PATCH 1/2] pager: move pager-specific setup into the build

I think you cannot > > really solve this without getting intimate with each pager (which people > > seemed not to want to do). > > Cooking pager specifics in git does sound bad. But it does not have to > be that way. What if we delegate the decision whether to color or n

Re: [PATCH 1/2] pager: move pager-specific setup into the build

On Mon, Aug 01, 2016 at 07:46:34PM +0200, Duy Nguyen wrote: > On Mon, Aug 1, 2016 at 3:05 AM, Eric Wong <e...@80x24.org> wrote: > > From: Junio C Hamano <gits...@pobox.com> > > > > Allowing PAGER_ENV to be set at build-time allows us to move > > pa

Re: [PATCH 2/2] pager: implement core.pagerEnv in config

On Mon, Aug 01, 2016 at 01:05:57AM +, Eric Wong wrote: > This allows overriding the build-time PAGER_ENV variable > at run-time. > > Inspired by part 1 of an idea from Kyle J. McKay at: > https://public-inbox.org/git/62db6def-8b39-4481-ba06-245bf4523...@gmail.com/ This commit message is

Re: [PATCH 1/2] pager: move pager-specific setup into the build

On Mon, Aug 1, 2016 at 3:05 AM, Eric Wong <e...@80x24.org> wrote: > From: Junio C Hamano <gits...@pobox.com> > > Allowing PAGER_ENV to be set at build-time allows us to move > pager-specific knowledge out of our build. Currently, this > allows us to set a better defau

Re: [PATCH 1/2] pager: move pager-specific setup into the build

On Mon, Aug 01, 2016 at 01:43:03AM +, brian m. carlson wrote: > On Mon, Aug 01, 2016 at 01:05:56AM +, Eric Wong wrote: > > +static void setup_pager_env(struct argv_array *env) > > +{ > > + const char *pager_env = stringify(PAGER_ENV); > > + > > + while (*pager_env) { > > +

Re: [PATCH 1/2] pager: move pager-specific setup into the build

; This is to handle environment variables holding program options, > which are usually (but possibly not often) using single character > options bundled together, that is, not using spaces. > > Moreover, it is about holding program options to pager. I understand that. My point is that we shoul

Re: [PATCH 1/2] pager: move pager-specific setup into the build

seeing how tools sometimes >> handle that poorly. This is to handle environment variables holding program options, which are usually (but possibly not often) using single character options bundled together, that is, not using spaces. Moreover, it is about holding program options to pager

Re: [PATCH 1/2] pager: move pager-specific setup into the build

"brian m. carlson" wrote: > On Mon, Aug 01, 2016 at 01:05:56AM +, Eric Wong wrote: > > + while (*cp && *cp == ' ') > > + cp++; > > So it looks like this function splits on spaces but doesn't provide any > escaping mechanism. Is there

Re: [PATCH 1/2] pager: move pager-specific setup into the build

On Mon, Aug 01, 2016 at 01:05:56AM +, Eric Wong wrote: > +static void setup_pager_env(struct argv_array *env) > +{ > + const char *pager_env = stringify(PAGER_ENV); > + > + while (*pager_env) { > + struct strbuf buf = STRBUF_INIT; > + const char *cp =

[PATCH 2/2] pager: implement core.pagerEnv in config

@ -456,4 +456,18 @@ test_expect_success 'command with underscores does not complain' ' test_cmp expect actual ' +test_expect_success TTY 'core.pagerEnv overrides build-time env' ' + ( + sane_unset LESS LV MORE && + git config core.pagerEnv MORE=-R &

[PATCH 1/2] pager: move pager-specific setup into the build

From: Junio C Hamano <gits...@pobox.com> Allowing PAGER_ENV to be set at build-time allows us to move pager-specific knowledge out of our build. Currently, this allows us to set a better default for FreeBSD where more(1) is the same binary as less(1). This also prepares us for introducing

Re: [PATCH] pager: disable color when pager is "more"

e: > >> > > 'git diff' outputs escape characters which clutter my terminal. Yes, I > >> > > can sed them out, but then why are they there? > >> > > >> > Those are most likely the ANSI sequences to add color. Can you call Git > >> > wit

Re: [PATCH] pager: disable color when pager is "more"

On Mon, Jul 18, 2016 at 10:19:09AM -0700, Junio C Hamano wrote: > > This is the tip of a smaller iceberg. See > > > > http://public-inbox.org/git/52D87A79.6060600%40rawbw.com/t/#u > > > > for more discussion, and some patches that fix more cases (like "LESS" > > without "R", or "more" that

Re: [PATCH] pager: disable color when pager is "more"

ch clutter my terminal. Yes, I >> > > can sed them out, but then why are they there? >> > >> > Those are most likely the ANSI sequences to add color. Can you call Git >> > with the --no-color option and see whether the escape characters go away? >> >

Re: [PATCH] pager: disable color when pager is "more"

t; can sed them out, but then why are they there? > > > > Those are most likely the ANSI sequences to add color. Can you call Git > > with the --no-color option and see whether the escape characters go away? > > Norm: do you have PAGER=more set by any chance? > Perhaps changing it to &q

[PATCH] pager: disable color when pager is "more"

to add color. Can you call Git > with the --no-color option and see whether the escape characters go away? Norm: do you have PAGER=more set by any chance? Perhaps changing it to "less" will allow you to preserve colors. I saw a similar or identical problem during my vacation in F

Re: [PATCH v2] Documentation: talk about pager in api-trace.txt

> trace_performance(t, "frotz"); > > + > +Bugs & Caveats > +-- > + > +GIT_TRACE_* environment variables can be used to tell Git to show > +trace output to its standard error stream. Git can often spawn a pager > +internally to run it

[PATCH v2] Documentation: talk about pager in api-trace.txt

tput to its standard error stream. Git can often spawn a pager +internally to run its subcommand and send its standard output and +standard error to it. + +Because GIT_TRACE_PERFORMANCE trace is generated only at the very end +of the program with atexit(), which happens after the pager exits, it

Re: [PATCH] Documentation: talk about pager in api-trace.txt

Christian Couder <christian.cou...@gmail.com> writes: > Junio do you plan to merge this patch or would you prefer something like: > > + > +Bugs & Caveats > +-- > + > +Some git commands, like `git log`, are run by default using a > +pager. In this case

Re: [PATCH] Documentation: talk about pager in api-trace.txt

erge this patch or would you prefer something like: + +Bugs & Caveats +-- + +Some git commands, like `git log`, are run by default using a +pager. In this case, stdout and stderr are redirected to the pager and +are closed when the pager exits. + +If a GIT_TRACE* environment variable

Re: [PATCH] Documentation: talk about pager in api-trace.txt

On Mon, Feb 29, 2016 at 03:21:20PM +0100, Christian Couder wrote: > Signed-off-by: Christian Couder > --- > Documentation/technical/api-trace.txt | 43 > +++ > 1 file changed, 43 insertions(+) I think this is fine. I'm not sure how many

Re: GIT_TRACE_PERFORMANCE and pager

On Mon, Feb 29, 2016 at 02:46:12PM +0100, Christian Couder wrote: > > though I guess I'd question whether trace-performance is interesting at > > all for a paged command, since it is also counting the length of time > > you spend looking at the pager waiting to hit

Re: GIT_TRACE_PERFORMANCE and pager

unting the length of time > you spend looking at the pager waiting to hit "q". Yup, exactly. I see Christian tried to add something to the documentation, but I think it is sufficient to suggest running commands with --no-pager. -- To unsubscribe from this list: send the line "un

[PATCH] Documentation: talk about pager in api-trace.txt

100644 --- a/Documentation/technical/api-trace.txt +++ b/Documentation/technical/api-trace.txt @@ -95,3 +95,46 @@ for (;;) { } trace_performance(t, "frotz"); + +Bugs & Caveats +-- + +Some git commands, like `git log`, are run by default using a +pager.

Re: GIT_TRACE_PERFORMANCE and pager

exited with 0 +++ > [pid 31155] +++ exited with 0 +++ > write(2, "06:32:30.486995 [pid=31154] trac"..., 114) = -1 EBADF (Bad file > descriptor) > write(2, "Could not trace into fd given by"..., 99) = -1 EBADF (Bad file > descriptor) > +++ exited with 0 +

Re: GIT_TRACE_PERFORMANCE and pager

file descriptor) write(2, "Could not trace into fd given by"..., 99) = -1 EBADF (Bad file descriptor) +++ exited with 0 +++ We redirect stderr to the pager (note that GIT_TRACE=1 still goes to stderr; it never goes to stdout). We wait() on the pager process before we exit the main

GIT_TRACE_PERFORMANCE and pager

Hi, It looks like setting GIT_TRACE_PERFORMANCE to 1 or 2 (for stdout or stderr) does not always work well with commands that use a pager, for example: - > GIT_TRACE_PERFORMANCE=2 git log -1 commit f02fbc4f9433937ee0463d0342d6d7d97e1f6f1e Author: Junio C Hamano <gits...@pob

[PATCH v2 2/3] pager: factor out a helper to prepare a child process to run the pager

When running a pager, we need to run the program git_pager() gave us, but we need to make sure we spawn it via the shell (i.e. it is valid to say PAGER='less -S', for example) and give default values to $LESS and $LV environment variables. Factor out these details to a separate helper function

[PATCH v2 1/3] pager: lose a separate argv[]

r(void) setenv("GIT_PAGER_IN_USE", "true", 1); /* spawn the pager */ - pager_argv[0] = pager; + argv_array_push(_process.args, pager); pager_process.use_shell = 1; - pager_process.argv = pager_argv; pager_process.in = -1;

Re: [PATCH 2/3] pager: factor out a helper to prepare a child process to run the pager

n the same page, but I am not sure how well that > interacts with what "git am -i" codepath wants to do, though. > > One big difference between the "we'll feed our output to pager" > codepath and "we'll spawn a pager to let a file on the filesystem be > read&

Re: [PATCH 2/3] pager: factor out a helper to prepare a child process to run the pager

Jeff King <p...@peff.net> writes: > On Tue, Feb 16, 2016 at 03:06:56PM -0800, Junio C Hamano wrote: > >> diff --git a/pager.c b/pager.c >> index 5dbcc5a..1406370 100644 >> --- a/pager.c >> +++ b/pager.c >> @@ -53,6 +53,23 @@ const char *git_pag

Re: [PATCH 2/3] pager: factor out a helper to prepare a child process to run the pager

On Tue, Feb 16, 2016 at 03:06:56PM -0800, Junio C Hamano wrote: > diff --git a/pager.c b/pager.c > index 5dbcc5a..1406370 100644 > --- a/pager.c > +++ b/pager.c > @@ -53,6 +53,23 @@ const char *git_pager(int stdout_is_tty) > return pager; > } > > +voi

[PATCH 2/3] pager: factor out a helper to prepare a child process to run the pager

When running a pager, we need to run the program git_pager() gave us, but we need to make sure we spawn it via the shell (i.e. it is valid to say PAGER='less -S', for example) and give default values to $LESS and $LV environment variables. Factor out these details to a separate helper function

[PATCH 1/3] pager: lose a separate argv[]

setenv("GIT_PAGER_IN_USE", "true", 1); /* spawn the pager */ - pager_argv[0] = pager; + argv_array_push(_process.args, pager); pager_process.use_shell = 1; - pager_process.argv = pager_argv; pager_process.in = -1;

Re: [PATCH 2/2] pager_in_use: make sure output is still going to pager

the same inode number as the pager pipe. Remember, inode numbers are only unique within a certain st_dev. /ceder -- 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

Re: [PATCH 2/2] pager_in_use: make sure output is still going to pager

Hi Peff, On 2015-08-10 07:23, Jeff King wrote: diff --git a/compat/pipe-id.c b/compat/pipe-id.c new file mode 100644 index 000..4764c5f --- /dev/null +++ b/compat/pipe-id.c @@ -0,0 +1,25 @@ +#include git-compat-util.h +#include compat/pipe-id.h +#include strbuf.h + +const char

Re: [PATCH 2/2] pager_in_use: make sure output is still going to pager

On Mon, Aug 10, 2015 at 06:38:10PM +0200, Johannes Schindelin wrote: +const char *pipe_id_get(int fd) +{ + static struct strbuf id = STRBUF_INIT; + struct stat st; + + if (fstat(fd, st) 0 || !S_ISFIFO(st.st_mode)) + return NULL; Just a quick note: it seems that

[PATCH 2/2] pager_in_use: make sure output is still going to pager

When we start a pager, we set GIT_PAGER_IN_USE=1 in the environment. This lets sub-processes know that even though isatty(1) is not true, it is because it is connected to a pager (and we should still turn on human-readable niceties like auto-color). Unfortunately, this is too inclusive

Re: [PATCH] pager: do not leak GIT_PAGER_IN_USE to the pager

On Fri, Jul 03, 2015 at 10:18:45AM -0700, Junio C Hamano wrote: Since 2e6c01e2, we export GIT_PAGER_IN_USE so that a process that I imagine you mean 2e6c012e here. becomes the upstream of the spawned pager can still tell that we have spawned the pager and decide to do colored output even

[PATCH] pager: do not leak GIT_PAGER_IN_USE to the pager

Since 2e6c01e2, we export GIT_PAGER_IN_USE so that a process that becomes the upstream of the spawned pager can still tell that we have spawned the pager and decide to do colored output even when its output no longer goes to a terminal (i.e. isatty(1)). But we forgot to clear it from

Re: [PATCH] git grep -O -i: if the pager is 'less', pass the '-i' option

Hi Junio, On Wed, 14 May 2014, Junio C Hamano wrote: Jonathan Nieder jrnie...@gmail.com writes: + if (opt.ignore_case !strcmp(less, pager)) + string_list_append(path_list, -i); I have a feeling that this goes against the recent trend of not mucking

Re: [PATCH] git grep -O -i: if the pager is 'less', pass the '-i' option

the lines of interest to the user. It would be nice if we could somehow point the pager at them by number, rather than repeating the (slightly incompatible) search. FWIW it is exactly that type of I want your patch to do more than you do type of comments that makes it impossible for myself

Re: [PATCH] git grep -O -i: if the pager is 'less', pass the '-i' option

Hi, Johannes Schindelin wrote: On Wed, 14 May 2014, Junio C Hamano wrote: Spot on. The change, especially with -I, makes sense. Except that it was not tested with -I. If you change it that way and it stops working on Windows, it's useless to me. Are you saying that less on Windows doesn't

Re: [PATCH] git grep -O -i: if the pager is 'less', pass the '-i' option

Johannes Schindelin wrote: On Wed, 14 May 2014, Jeff King wrote: We've already found the lines of interest to the user. It would be nice if we could somehow point the pager at them by number, rather than repeating the (slightly incompatible) search. FWIW it is exactly that type of I want

Re: [PATCH] git grep -O -i: if the pager is 'less', pass the '-i' option

Hi, On Thu, 15 May 2014, Jonathan Nieder wrote: Johannes Schindelin wrote: On Wed, 14 May 2014, Junio C Hamano wrote: Spot on. The change, especially with -I, makes sense. Except that it was not tested with -I. If you change it that way and it stops working on Windows, it's useless

Re: [PATCH] git grep -O -i: if the pager is 'less', pass the '-i' option

On Thu, May 15, 2014 at 07:46:49PM +0200, Johannes Schindelin wrote: We've already found the lines of interest to the user. It would be nice if we could somehow point the pager at them by number, rather than repeating the (slightly incompatible) search. FWIW it is exactly that type of I

Re: [PATCH] git grep -O -i: if the pager is 'less', pass the '-i' option

Johannes Schindelin johannes.schinde...@gmx.de writes: Jonathan Nieder jrnie...@gmail.com writes: +if (opt.ignore_case !strcmp(less, pager)) +string_list_append(path_list, -i); I have a feeling that this goes against the recent trend

[PATCH] git grep -O -i: if the pager is 'less', pass the '-i' option

changed, 3 insertions(+) diff --git a/builtin/grep.c b/builtin/grep.c index 8073fbe..6c82a29 100644 --- a/builtin/grep.c +++ b/builtin/grep.c @@ -897,6 +897,9 @@ int cmd_grep(int argc, const char **argv, const char *prefix) if (len 4 is_dir_sep(pager[len - 5

Re: [PATCH] git grep -O -i: if the pager is 'less', pass the '-i' option

is_dir_sep(pager[len - 5])) pager += len - 4; + if (opt.ignore_case !strcmp(less, pager)) + string_list_append(path_list, -i); I have a feeling that this goes against the recent trend of not mucking with the expectation of the users

Re: [PATCH] git grep -O -i: if the pager is 'less', pass the '-i' option

char *prefix) if (len 4 is_dir_sep(pager[len - 5])) pager += len - 4; +if (opt.ignore_case !strcmp(less, pager)) +string_list_append(path_list, -i); I have a feeling that this goes against the recent trend of not mucking

Re: [PATCH] git grep -O -i: if the pager is 'less', pass the '-i' option

Hi, Junio C Hamano wrote: Stepan Kasal ka...@ucw.cz writes: --- a/builtin/grep.c +++ b/builtin/grep.c @@ -897,6 +897,9 @@ int cmd_grep(int argc, const char **argv, const char *prefix) if (len 4 is_dir_sep(pager[len - 5])) pager += len - 4

Re: [PATCH] git grep -O -i: if the pager is 'less', pass the '-i' option

Hello, On Wed, May 14, 2014 at 11:26:54AM -0700, Jonathan Nieder wrote: But as is, it's an improvement, so (except that -i should be replaced by -I) it seems like a good change. indeed, -I would match the semantics of git-grep -i . Stepan -- To unsubscribe from this list: send the line

Re: [PATCH] git grep -O -i: if the pager is 'less', pass the '-i' option

Jonathan Nieder jrnie...@gmail.com writes: + if (opt.ignore_case !strcmp(less, pager)) + string_list_append(path_list, -i); I have a feeling that this goes against the recent trend of not mucking with the expectation of the users on their pagers, if I recall

Re: [PATCH] git grep -O -i: if the pager is 'less', pass the '-i' option

somehow point the pager at them by number, rather than repeating the (slightly incompatible) search. We can do less +25, but that only points it to the first instance (and doesn't highlight it), whereas the current code lets the repeat-search find other instances. I don't think there's a way

Re: [PATCH v2] pager: remove 'S' from $LESS by default

Jeff King p...@peff.net writes: On Tue, May 06, 2014 at 08:49:22PM +0200, Matthieu Moy wrote: ... The idea of having a separate default value for pager.blame (or set $LESS differently for blame) crossed my mind, but I actually don't like it, as it would make it harder for a user to fine-tune

Re: [PATCH v2] pager: remove 'S' from $LESS by default

Junio C Hamano gits...@pobox.com writes: While I fully agree with the above conclusion, I just noticed that I will be irritated enough to eventually set pager.blame myself, after running a short git blame -L1311,+7 git-p4.py, which is one of the standard first steps for me to start reading

Re: [PATCH v2] pager: remove 'S' from $LESS by default

Matthieu Moy matthieu@grenoble-inp.fr writes: Perhaps it deserves a mention in the doc, e.g. squashing this on top of my patch: Thanks, will do. diff --git a/Documentation/config.txt b/Documentation/config.txt index b7f92ac..ebd1676 100644 --- a/Documentation/config.txt +++

Re: [PATCH v2] pager: remove 'S' from $LESS by default

Matthieu Moy matthieu@grenoble-inp.fr writes: By default, Git used to set $LESS to -FRSX if $LESS was not set by the user. The FRX flags actually make sense for Git (F and X because Git sometimes pipes short output to less, and R because Git pipes colored output). The S flag (chop

Re: [PATCH v2] pager: remove 'S' from $LESS by default

blame sucks in anything but fullscreen either way. It would help to display _only_ the source code and have the other info as mouse-over, but that's not something a pager can do. It is a pity that the content can be columnized much worse than the metadata: otherwise it would make much more sense

Re: [PATCH v2] pager: remove 'S' from $LESS by default

are longer than my window width. git blame sucks in anything but fullscreen either way. It would help to display _only_ the source code and have the other info as mouse-over, but that's not something a pager can do. Exactly. I personally never use git blame outside git gui blame for this reason

Re: [PATCH v2] pager: remove 'S' from $LESS by default

On Tue, May 06, 2014 at 08:49:22PM +0200, Matthieu Moy wrote: Exactly. I personally never use git blame outside git gui blame for this reason. I'd recommend tig blame for this, too, which behaves like less -S with respect to long lines (and also makes it easy to jump to the full diff, or

Re: [PATCH v2] pager: remove 'S' from $LESS by default

Hi, Matthieu Moy wrote: By default, Git used to set $LESS to -FRSX if $LESS was not set by the user. The FRX flags actually make sense for Git (F and X because Git sometimes pipes short output to less, and R because Git pipes colored output). The S flag (chop long lines), on the other hand,

Re: [PATCH v2] pager: remove 'S' from $LESS by default

- Original Message - Hi, Matthieu Moy wrote: By default, Git used to set $LESS to -FRSX if $LESS was not set by the user. The FRX flags actually make sense for Git (F and X because Git sometimes pipes short output to less, and R because Git pipes colored output). The S flag

[PATCH v2] pager: remove 'S' from $LESS by default

and is a matter of user preference. Git should not decide for the user to change LESS's default. More specifically, the S flag harms users who review untrusted code within a pager, since a patch looking like: -old code; +new good code; [... lots of tabs ...] malicious code; would appear identical

Re: [PATCH v2] pager: remove 'S' from $LESS by default

. Thanks. More specifically, the S flag harms users who review untrusted code within a pager, since a patch looking like: -old code; +new good code; [... lots of tabs ...] malicious code; would appear identical to: -old code; +new good code; Users who prefer the old behavior can still

Re: [PATCH v2] pager: remove 'S' from $LESS by default

output). The S flag (chop long lines), on the other hand, is not related to Git and is a matter of user preference. Git should not decide for the user to change LESS's default. Git always pipes its output to less, Err, no, not all commands use a pager. I am inclined to suggest queuing

Re: [PATCH v2] pager: remove 'S' from $LESS by default

use a pager. Yeah, what I meant to say was that when it is told to use pager (either by an explicit git -p or implicitly with the lack of git --no-pager), it does not count the lines shown to avoid passing short output to the pager. sometimes pipes sounded to me as if we attempt to do so

Re: [PATCH] Revert Stop starting pager recursively

On Mon, 28 April 2014 10:14:05 -0700, Junio C Hamano wrote: Matthieu Moy matthieu@grenoble-inp.fr writes: - Original Message - On Sun, Apr 27, 2014 at 09:12:39AM +0700, Duy Nguyen wrote: The intent of the commit was that is a stupid thing to do, but it's not so obvious

Re: [PATCH] Revert Stop starting pager recursively

be very much appreciated. Thanks. -- 8 -- From: Jörn Engel jo...@logfs.org Date: Mon, 21 Apr 2014 16:46:22 -0400 Subject: [PATCH] pager: do allow spawning pager recursively This reverts commit 88e8f908f2b0c56f9ccf8134d8ff9f689af9cc84, which tried to allow GIT_PAGER=git -p column --mode='dense

Re: [PATCH] Revert Stop starting pager recursively

On Mon, 28 April 2014 16:04:28 -0700, Junio C Hamano wrote: Just the Sign-off is trivial enough that even this brainless patch-monkey^Wpanda should be able to handle. The part The log message could be improved is something you may be better equipped to, though. Looks good to me. The next

Re: [PATCH] Revert Stop starting pager recursively

On Sun, Apr 27, 2014 at 09:12:39AM +0700, Duy Nguyen wrote: The intent of the commit was that is a stupid thing to do, but it's not so obvious from the first glance, do not freeze my system for my mistake. But if it stops an actual use case, then I agree it should be reverted. Thanks for the

<    1   2   3   >