Re: [PATCH] PAGER_ENV: remove 'S' from $LESS by default
Matthieu Moy writes: >> I also agree that droppage of S does not have to wait for that >> topic. > > So, shall I rewrite my patch on top of master? (not hard, but there will > be a minor conflict to resolve when merging with Peff's cooking series). Sure, the one near the tip of 'pu' can even be dropped, especially when nobody is actively looking at it, if it turns out to be too much of a nuisance. Thanks. -- 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] PAGER_ENV: remove 'S' from $LESS by default
Junio C Hamano writes: > Jeff King writes: > >> On Mon, Apr 28, 2014 at 02:22:21PM +0200, Matthieu Moy wrote: >> >> I'd be OK with doing the moral equivalent for now (perhaps just taking >> Junio's proposal[1]), and I can deal with the refactoring later when >> re-rolling the Makefile series. >> >> -Peff >> >> [1] http://article.gmane.org/gmane.comp.version-control.git/240637 > > I doubt we would want to use the patch verbatim in that message; it > served its purpose well to illustrate that there may be other ways > to address the issue, but I agreed with the flaw in it you pointed > out in the thread [*1*] > > I also agree that droppage of S does not have to wait for that > topic. So, shall I rewrite my patch on top of master? (not hard, but there will be a minor conflict to resolve when merging with Peff's cooking series). -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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] PAGER_ENV: remove 'S' from $LESS by default
Jeff King writes: > On Mon, Apr 28, 2014 at 02:22:21PM +0200, Matthieu Moy wrote: > > I'd be OK with doing the moral equivalent for now (perhaps just taking > Junio's proposal[1]), and I can deal with the refactoring later when > re-rolling the Makefile series. > > -Peff > > [1] http://article.gmane.org/gmane.comp.version-control.git/240637 I doubt we would want to use the patch verbatim in that message; it served its purpose well to illustrate that there may be other ways to address the issue, but I agreed with the flaw in it you pointed out in the thread [*1*] I also agree that droppage of S does not have to wait for that topic. [Reference] *1* http://thread.gmane.org/gmane.comp.version-control.git/240548/focus=240746 -- 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] PAGER_ENV: remove 'S' from $LESS by default
On Mon, Apr 28, 2014 at 02:22:21PM +0200, Matthieu Moy wrote: > >> Not since f82c3ffd862c7 (Wed Feb 5 2014, move LESS/LV pager environment > >> to Makefile). > > > > The only upstream branch containing this commit is pu. So this patch > > should likely not go anywhere else for now. > > Oops, indeed, I made my patch on top of pu by mistake. Anyway, my patch > can wait for the other series to be merged. > > Jeff, you're the author of f82c3ffd862c7, topic jk/makefile in git.git, > marked "expecting a reroll" by Junio. Any news from the series? I am planning to revisit it eventually, but it's fairly low priority. There is some pretty heavy refactoring in the series, and the PAGER_ENV bits do not have to be held hostage to that refactoring (they are really just demonstrating the refactoring). I'd be OK with doing the moral equivalent for now (perhaps just taking Junio's proposal[1]), and I can deal with the refactoring later when re-rolling the Makefile series. -Peff [1] http://article.gmane.org/gmane.comp.version-control.git/240637 -- 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] PAGER_ENV: remove 'S' from $LESS by default
David Kastrup writes: > Matthieu Moy writes: > >> David Kastrup writes: >> >>> There seem to be a few more occurences (git-sh-setup.sh and pager.c): >> >> Not since f82c3ffd862c7 (Wed Feb 5 2014, move LESS/LV pager environment >> to Makefile). > > The only upstream branch containing this commit is pu. So this patch > should likely not go anywhere else for now. Oops, indeed, I made my patch on top of pu by mistake. Anyway, my patch can wait for the other series to be merged. Jeff, you're the author of f82c3ffd862c7, topic jk/makefile in git.git, marked "expecting a reroll" by Junio. Any news from the series? -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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] PAGER_ENV: remove 'S' from $LESS by default
Matthieu Moy writes: > David Kastrup writes: > >> There seem to be a few more occurences (git-sh-setup.sh and pager.c): > > Not since f82c3ffd862c7 (Wed Feb 5 2014, move LESS/LV pager environment > to Makefile). The only upstream branch containing this commit is pu. So this patch should likely not go anywhere else for now. -- David Kastrup -- 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] PAGER_ENV: remove 'S' from $LESS by default
David Kastrup writes: > There seem to be a few more occurences (git-sh-setup.sh and pager.c): Not since f82c3ffd862c7 (Wed Feb 5 2014, move LESS/LV pager environment to Makefile). > Searching for LESS seems to implicate a few more possible candidates in > contrib/examples: > > contrib/examples/git-log.sh:LESS=-S ${PAGER:-less} > contrib/examples/git-whatchanged.sh:LESS="$LESS -S" ${PAGER:-less} Yes, I did see these, but I considered that contrib/examples/ should remain a snapshot of what the commands used to look like at the time they were shell scripts. There's also user-manual.txt: , | Basically, the initial version of `git log` was a shell script: | | | $ git-rev-list --pretty $(git-rev-parse --default HEAD "$@") | \ | LESS=-S ${PAGER:-less} | ` that I left intact. I can change them too if people prefer. Thanks, -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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] PAGER_ENV: remove 'S' from $LESS by default
Matthieu Moy 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 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. >> We agree here. So, does someone who actually wants this change want to >> propose a patch? :) > > Here you are. > > Documentation/config.txt | 13 - > Makefile | 6 +++--- > perl/Git/SVN/Log.pm | 2 +- > 3 files changed, 12 insertions(+), 9 deletions(-) There seem to be a few more occurences (git-sh-setup.sh and pager.c): $ git grep FRSX Documentation/RelNotes/1.6.5.txt: * mingw will also give FRSX as the default val Documentation/config.txt:When the `LESS` environment variable is unset, Git sets Documentation/config.txt:command to `LESS=FRSX less -+S`. The environment tells git-sh-setup.sh:: ${LESS=-FRSX} pager.c:env[i++] = "LESS=FRSX"; perl/Git/SVN/Log.pm:$ENV{LESS} ||= 'FRSX'; Searching for LESS seems to implicate a few more possible candidates in contrib/examples: contrib/examples/git-log.sh:LESS=-S ${PAGER:-less} contrib/examples/git-whatchanged.sh:LESS="$LESS -S" ${PAGER:-less} -- David Kastrup -- 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