{"thread":{"id":"36485","subject":"Harmful LESS flags","startedAt":"2014-04-23T22:44:03Z","lastAt":"2014-05-07T20:42:02Z","messageCount":41,"participants":["d9ba@mailtor.net","Jonathan Nieder","David Kastrup","Junio C Hamano","Jeff King","Matthieu Moy","Mark Nudelman"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"239507","messageId":"4dc69237123e8962b2b2b901692ea78e.id@mailtor","threadId":"36485","inReplyTo":null,"subject":"Harmful LESS flags","fromName":"","fromEmail":"d9ba@mailtor.net","sentAt":null,"receivedAt":"2014-04-23T22:44:03Z","isPatch":false,"sender":{"key":"d9ba@mailtor.net","avatar":null},"body":"hello list,\n\nas mentioned earlier on IRC, I'm a bit concerned about the default LESS flags\nused by git.\n\nThe S option causes git to cut off everything to the right\n\nConsider this diff, printed by `git diff`\n\n\t #!/usr/bin/env python\n\t-print('foo')\n\t+print('bar')\n\nLooks ok to merge and run.\n\nBut, after disabling the pager:\n\n\t #!/usr/bin/env python\n\t-print('foo')\n\t+print('bar') [lots of tabs] ; import os; os.system('aptitude install\nsubversion')\n\nOh no!\n\nMy workflow is to clone a project, read the whole source and review all diffs\nafter fetching them. After that is done I merge origin into my local\nbranch and\nrun the code on my system.\n\nI've panic'd a bit after I've noticed the chopping.\n\nIt would be nice if we could change the flags to either\n\n a) avoid cutting off\n b) indicate something has been cut off (<- I prefer this)\n\nI assume there are more people with a similar workflow who're still\nunaware of\nthis feature.\n\nI would joke about how 3 letter agencies introduced this flag to backdoor\nopen\nsource projects, but, well..\n\n\tSincerely yours,\n\ta git user\n"},{"id":"239508","messageId":"20140424001126.GG15516@google.com","threadId":"36485","inReplyTo":"4dc69237123e8962b2b2b901692ea78e.id@mailtor","subject":"Re: Harmful LESS flags","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2014-04-24T00:11:26Z","receivedAt":"2014-04-24T00:11:26Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"(cc-ing Mark Nudelman, less maintainer)\nHi,\n\nd9ba@mailtor.net wrote:\n\n> Consider this diff, printed by `git diff`\n>\n> \t #!/usr/bin/env python\n> \t-print('foo')\n> \t+print('bar')\n>\n> Looks ok to merge and run.\n>\n> But, after disabling the pager:\n\nUnfortunately there are other kinds of subtle bugs that can be hard to\nsee in a terminal, too.\n\n[...]\n> It would be nice if we could change the flags to either\n>\n>  a) avoid cutting off\n>  b) indicate something has been cut off (<- I prefer this)\n\nThat sounds like a nice feature request for 'less': a marker on the\nright margin when --chop-long-lines is in use and a line has been\nchopped.  I don't see it at\nhttp://www.greenwoodsoftware.com/less/bugs.html#enhance so maybe no\none else has thought of it yet.\n\nMark, what do you think?\n\nThanks,\nJonathan\n"},{"id":"239535","messageId":"87lhuvb9kr.fsf@fencepost.gnu.org","threadId":"36485","inReplyTo":"4dc69237123e8962b2b2b901692ea78e.id@mailtor","subject":"Re: Harmful LESS flags","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2014-04-24T05:06:12Z","receivedAt":"2014-04-24T05:06:12Z","isPatch":false,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"d9ba@mailtor.net writes:\n\n> It would be nice if we could change the flags to either\n>\n>  a) avoid cutting off\n>  b) indicate something has been cut off (<- I prefer this)\n>\n> I assume there are more people with a similar workflow who're still\n> unaware of this feature.\n>\n> I would joke about how 3 letter agencies introduced this flag to\n> backdoor open source projects, but, well..\n\nMost terminals are wider than three letters.\n\nStill, it is a total nuisance.  I am constantly doing\n\n-S RET\n\non my git output.  This should be left alone as an entirely personal\npreference quite unrelated to Git.  There is no point in having Git\nconfigure a default different from what is used elsewhere.\n\n-- \nDavid Kastrup\n"},{"id":"239594","messageId":"xmqqha5iv9eb.fsf@gitster.dls.corp.google.com","threadId":"36485","inReplyTo":"87lhuvb9kr.fsf@fencepost.gnu.org","subject":"Re: Harmful LESS flags","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-04-24T19:02:04Z","receivedAt":"2014-04-24T19:02:04Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Kastrup <dak@gnu.org> writes:\n\n> d9ba@mailtor.net writes:\n>\n>> It would be nice if we could change the flags to either\n>>\n>>  a) avoid cutting off\n>>  b) indicate something has been cut off (<- I prefer this)\n>>\n>> I assume there are more people with a similar workflow who're still\n>> unaware of this feature.\n>>\n>> I would joke about how 3 letter agencies introduced this flag to\n>> backdoor open source projects, but, well..\n>\n> Most terminals are wider than three letters.\n\nI am having a hard time to decide if you genuinely misread what you\nare responding to, or if you are joking.  If the latter, I find the\njoke mildly funny in a twisted way ;-)\n\nBut the tangent aside...\n\n> Still, it is a total nuisance.  I am constantly doing\n>\n> -S RET\n>\n> on my git output.  This should be left alone as an entirely personal\n> preference quite unrelated to Git.  There is no point in having Git\n> configure a default different from what is used elsewhere.\n\nI almost agree with the general principle of the last sentence, but\nwith a bit of reservation.  The default value for LESS (i.e. when\nthe user does not have any) we pass is FRSX, and the Porcelain\noutput these days is colored by default.  If we don't set a default\nat all, the end-user experience for a newbie will be bad, especially\nwithout \"R\".\n\nAmong the other three, F and X are to avoid a short output (e.g \"git\nshow\" on a one-liner with a short explanation) from asking for\nconfirmation to leave the pager and from clearing the screen upon\nleaving the pager, and are generally accepted as good things (or at\nleast, we haven't seen much issue raised after we started passing\nthe default LESS for those who do not have their own in their\nenvironment).\n\nUse of S is very subjective.  While I personally do appreciate that\nwe have it by default, I can perfectly well understand why some\npeople do not want to see it in the default.  The best we can do is\nto arrange so that people from one of the camps have their favorite\nout of the box and those from the other camp need to tell Git that\nthey want to (or do not want to) fold long lines.\n\nTraditionally, because the tool grew in a context of being used in a\nproject whose participants are at least not malicious, always having\nto be on the lookout for fear of middle-of-line tabs hiding bad\ncontents near the right edges of lines has never been an issue.  If\nsomebody brought up a potential issue of such mode of attack back\nthen, Linus may have chosen the default differently.  I may have\nmyself chosen not to have S, if I were the maintainer when the LESS\ndefault was originally introduced, and had I been made aware of this\nissue.\n\nI am not opposed to changing the default in the longer term, as long\nas we have a solid transition plan to ensure that it won't disrupt\nand/or upset existing users too much.\n"},{"id":"239598","messageId":"87tx9ia5zq.fsf@fencepost.gnu.org","threadId":"36485","inReplyTo":"xmqqha5iv9eb.fsf@gitster.dls.corp.google.com","subject":"Re: Harmful LESS flags","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2014-04-24T19:21:13Z","receivedAt":"2014-04-24T19:21:13Z","isPatch":false,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Traditionally, because the tool grew in a context of being used in a\n> project whose participants are at least not malicious, always having\n> to be on the lookout for fear of middle-of-line tabs hiding bad\n> contents near the right edges of lines has never been an issue.\n\nMy beef is not with \"hiding bad contents\" but with \"hiding contents\".\nIt makes the output useless for seeing what is actually happening as\nsoon as the option starts having an effect.\n\n-- \nDavid Kastrup\n"},{"id":"239600","messageId":"xmqq8uquv84u.fsf@gitster.dls.corp.google.com","threadId":"36485","inReplyTo":"87tx9ia5zq.fsf@fencepost.gnu.org","subject":"Re: Harmful LESS flags","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-04-24T19:29:21Z","receivedAt":"2014-04-24T19:29:21Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Kastrup <dak@gnu.org> writes:\n\n> Junio C Hamano <gitster@pobox.com> writes:\n>\n>> Traditionally, because the tool grew in a context of being used in a\n>> project whose participants are at least not malicious, always having\n>> to be on the lookout for fear of middle-of-line tabs hiding bad\n>> contents near the right edges of lines has never been an issue.\n>\n> My beef is not with \"hiding bad contents\" but with \"hiding contents\".\n> It makes the output useless for seeing what is actually happening as\n> soon as the option starts having an effect.\n\nMy suspicion is that one of the reasons why S was chosen to be in\nthe default was to mildly discourage people from busting the usual\nline-length limit, but I am not Linus ;-)\n"},{"id":"239604","messageId":"87ppk6a4nl.fsf@fencepost.gnu.org","threadId":"36485","inReplyTo":"xmqq8uquv84u.fsf@gitster.dls.corp.google.com","subject":"Re: Harmful LESS flags","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2014-04-24T19:50:06Z","receivedAt":"2014-04-24T19:50:06Z","isPatch":false,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> David Kastrup <dak@gnu.org> writes:\n>\n>> Junio C Hamano <gitster@pobox.com> writes:\n>>\n>>> Traditionally, because the tool grew in a context of being used in a\n>>> project whose participants are at least not malicious, always having\n>>> to be on the lookout for fear of middle-of-line tabs hiding bad\n>>> contents near the right edges of lines has never been an issue.\n>>\n>> My beef is not with \"hiding bad contents\" but with \"hiding contents\".\n>> It makes the output useless for seeing what is actually happening as\n>> soon as the option starts having an effect.\n>\n> My suspicion is that one of the reasons why S was chosen to be in\n> the default was to mildly discourage people from busting the usual\n> line-length limit, but I am not Linus ;-)\n\nExcept that the busting becomes less rather than more conspicuous.\n\n-- \nDavid Kastrup\n"},{"id":"239607","messageId":"20140424213529.GB7815@sigill.intra.peff.net","threadId":"36485","inReplyTo":"xmqq8uquv84u.fsf@gitster.dls.corp.google.com","subject":"Re: Harmful LESS flags","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-04-24T21:35:29Z","receivedAt":"2014-04-24T21:35:29Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Apr 24, 2014 at 12:29:21PM -0700, Junio C Hamano wrote:\n\n> David Kastrup <dak@gnu.org> writes:\n> \n> > Junio C Hamano <gitster@pobox.com> writes:\n> >\n> >> Traditionally, because the tool grew in a context of being used in a\n> >> project whose participants are at least not malicious, always having\n> >> to be on the lookout for fear of middle-of-line tabs hiding bad\n> >> contents near the right edges of lines has never been an issue.\n> >\n> > My beef is not with \"hiding bad contents\" but with \"hiding contents\".\n> > It makes the output useless for seeing what is actually happening as\n> > soon as the option starts having an effect.\n> \n> My suspicion is that one of the reasons why S was chosen to be in\n> the default was to mildly discourage people from busting the usual\n> line-length limit, but I am not Linus ;-)\n\nI would think it's the opposite. Long lines look _horrible_ without\n\"-S\", as they get wrapped at awkward points. Using \"-S\" means that long\nlines don't bug you, unless you really want to scroll over and see the\ncontent.\n\nI really think the right solution here is to teach less to make it more\nobvious that there is something worth scrolling over to. Here's a very\nrough patch for less, if you want to see what I'm thinking of.\n\ndiff --git a/input.c b/input.c\nindex b211323..01aa411 100755\n--- a/input.c\n+++ b/input.c\n@@ -178,6 +178,7 @@ get_forw_line:\n \t\t\t */\n \t\t\tif (chopline || hshift > 0)\n \t\t\t{\n+\t\t\t\tset_chopped_marker(ch_tell()-1);\n \t\t\t\tdo\n \t\t\t\t{\n \t\t\t\t\tif (ABORT_SIGS())\ndiff --git a/line.c b/line.c\nindex 1eb3914..b3358a0 100755\n--- a/line.c\n+++ b/line.c\n@@ -1080,6 +1080,20 @@ set_status_col(c)\n \tattr[0] = AT_NORMAL|AT_HILITE;\n }\n \n+\tpublic void\n+set_chopped_marker(pos)\n+\t    POSITION pos;\n+{\n+\t/*\n+\t * Roll back output by one character; probably\n+\t * we need to actually walk curr back further\n+\t * for multibyte characters?\n+\t */\n+\tcolumn--;\n+\tcurr--;\n+\tstore_char('>', AT_NORMAL|AT_HILITE, NULL, pos);\n+}\n+\n /*\n  * Get a character from the current line.\n  * Return the character as the function return value,\n\n-Peff\n"},{"id":"239609","messageId":"xmqq4n1iv1re.fsf@gitster.dls.corp.google.com","threadId":"36485","inReplyTo":"20140424213529.GB7815@sigill.intra.peff.net","subject":"Re: Harmful LESS flags","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-04-24T21:47:01Z","receivedAt":"2014-04-24T21:47:01Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> I would think it's the opposite. Long lines look _horrible_ without\n> \"-S\", as they get wrapped at awkward points. Using \"-S\" means that long\n> lines don't bug you, unless you really want to scroll over and see the\n> content.\n>\n> I really think the right solution here is to teach less to make it more\n> obvious that there is something worth scrolling over to. Here's a very\n> rough patch for less, if you want to see what I'm thinking of.\n\nYes, I think that was suggested as an issue worth bringing up with\nless maintainers earlier in the thread already (and that was why I\ndidn't repeat it).  If we were in the business of updating less to\nsuit many users' needs (the needs of our users included), we may\neven want to advocate turning R on by default.\n\nAnd I do agree that the \"chopped marker\" would be a very sensible\nthing to show in the \"-S\" output; I would have chosen \"$\" myself for\nthat to match an existing practice in (setq truncate-lines t) in\nEmacs, though.\n"},{"id":"239611","messageId":"87lhuu9z69.fsf@fencepost.gnu.org","threadId":"36485","inReplyTo":"20140424213529.GB7815@sigill.intra.peff.net","subject":"Re: Harmful LESS flags","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2014-04-24T21:48:30Z","receivedAt":"2014-04-24T21:48:30Z","isPatch":false,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Thu, Apr 24, 2014 at 12:29:21PM -0700, Junio C Hamano wrote:\n>\n>> David Kastrup <dak@gnu.org> writes:\n>> \n>> > Junio C Hamano <gitster@pobox.com> writes:\n>> >\n>> >> Traditionally, because the tool grew in a context of being used in a\n>> >> project whose participants are at least not malicious, always having\n>> >> to be on the lookout for fear of middle-of-line tabs hiding bad\n>> >> contents near the right edges of lines has never been an issue.\n>> >\n>> > My beef is not with \"hiding bad contents\" but with \"hiding contents\".\n>> > It makes the output useless for seeing what is actually happening as\n>> > soon as the option starts having an effect.\n>> \n>> My suspicion is that one of the reasons why S was chosen to be in\n>> the default was to mildly discourage people from busting the usual\n>> line-length limit, but I am not Linus ;-)\n>\n> I would think it's the opposite. Long lines look _horrible_ without\n> \"-S\", as they get wrapped at awkward points. Using \"-S\" means that long\n> lines don't bug you, unless you really want to scroll over and see the\n> content.\n\nI prefer horrible over useless.\n\n> I really think the right solution here is to teach less to make it more\n> obvious that there is something worth scrolling over to. Here's a very\n> rough patch for less, if you want to see what I'm thinking of.\n\nStill useless.  I'm not actually interested in a more prominent \"I could\nbe useful\" indicator.\n\n-- \nDavid Kastrup\n"},{"id":"239610","messageId":"20140424220241.GC7815@sigill.intra.peff.net","threadId":"36485","inReplyTo":"xmqq4n1iv1re.fsf@gitster.dls.corp.google.com","subject":"Re: Harmful LESS flags","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-04-24T22:02:41Z","receivedAt":"2014-04-24T22:02:41Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Apr 24, 2014 at 02:47:01PM -0700, Junio C Hamano wrote:\n\n> And I do agree that the \"chopped marker\" would be a very sensible\n> thing to show in the \"-S\" output; I would have chosen \"$\" myself for\n> that to match an existing practice in (setq truncate-lines t) in\n> Emacs, though.\n\nHmm. I do not use Emacs, but I explicitly avoided \"$\" because of its\nend-of-line connotations. E.g., in \"cat -A\" it means the opposite: this\nis the real \"\\n\" end-of-line. But if there's existing precedent for \"$\",\nthat would be fine with me.\n\n-Peff\n"},{"id":"239613","messageId":"20140424221308.GA15061@sigill.intra.peff.net","threadId":"36485","inReplyTo":"87lhuu9z69.fsf@fencepost.gnu.org","subject":"Re: Harmful LESS flags","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-04-24T22:13:08Z","receivedAt":"2014-04-24T22:13:08Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Apr 24, 2014 at 11:48:30PM +0200, David Kastrup wrote:\n\n> > I really think the right solution here is to teach less to make it more\n> > obvious that there is something worth scrolling over to. Here's a very\n> > rough patch for less, if you want to see what I'm thinking of.\n> \n> Still useless.  I'm not actually interested in a more prominent \"I could\n> be useful\" indicator.\n\nSo don't set -S, then.\n\nThere are two questions here:\n\n  1. Can less do a better job of indicating what's in the input when -S\n     is in effect?\n\n  2. What should get put into $LESS by default?\n\nI was specifically addressing (1). Your comment does not help at all\nthere.\n\nIt could have an impact on (2), but you didn't say anything besides \"I\ndon't like it\". That doesn't add anything to the conversation.\n\n-Peff\n"},{"id":"239614","messageId":"87ha5i9wkp.fsf@fencepost.gnu.org","threadId":"36485","inReplyTo":"20140424221308.GA15061@sigill.intra.peff.net","subject":"Re: Harmful LESS flags","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2014-04-24T22:44:38Z","receivedAt":"2014-04-24T22:44:38Z","isPatch":false,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Thu, Apr 24, 2014 at 11:48:30PM +0200, David Kastrup wrote:\n>\n>> > I really think the right solution here is to teach less to make it more\n>> > obvious that there is something worth scrolling over to. Here's a very\n>> > rough patch for less, if you want to see what I'm thinking of.\n>> \n>> Still useless.  I'm not actually interested in a more prominent \"I could\n>> be useful\" indicator.\n>\n> So don't set -S, then.\n\nI don't.  Git does it unasked for.\n\n> There are two questions here:\n>\n>   1. Can less do a better job of indicating what's in the input when -S\n>      is in effect?\n>\n>   2. What should get put into $LESS by default?\n>\n> I was specifically addressing (1). Your comment does not help at all\n> there.\n>\n> It could have an impact on (2), but you didn't say anything besides \"I\n> don't like it\". That doesn't add anything to the conversation.\n\nNo, I said it is useless, which is different from \"I don't like it\".\nThe information is not copy&pastable from a terminal window since it is\ncut off.  It is also useless for review since one does not actually know\nwhat's in there.  The only thing it has going for it is that it's\nprettier than the actually usable information.  Which might sometimes be\nnice if one is not interested overly in the payload, like when using\n--graph.  But then even a graph display wants to get copy&pasted into\nwindows with a different size from the terminal window, like in\n<URL:http://code.google.com/p/lilypond/issues/detail?id=3723#c7>.\n\n-- \nDavid Kastrup\n"},{"id":"239620","messageId":"20140424230812.GM15516@google.com","threadId":"36485","inReplyTo":"87ha5i9wkp.fsf@fencepost.gnu.org","subject":"Re: Harmful LESS flags","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2014-04-24T23:08:12Z","receivedAt":"2014-04-24T23:08:12Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nDavid Kastrup wrote:\n> Jeff King <peff@peff.net> writes:\n\n>> There are two questions here:\n>>\n>>   1. Can less do a better job of indicating what's in the input when -S\n>>      is in effect?\n>>\n>>   2. What should get put into $LESS by default?\n>>\n>> I was specifically addressing (1). Your comment does not help at all\n>> there.\n>>\n>> It could have an impact on (2), but you didn't say anything besides \"I\n>> don't like it\". That doesn't add anything to the conversation.\n>\n> No, I said it is useless, which is different from \"I don't like it\".\n> The information is not copy&pastable from a terminal window since it is\n> cut off.  It is also useless for review since one does not actually know\n> what's in there.  The only thing it has going for it is that it's\n> prettier than the actually usable information.\n\nI disagree with your characterization of what's useful here, but it\nreally doesn't matter.  Why are you still arguing?\n\nI think it would be fine to change git's default for LESS to FRX and\ndocument that change wherever the documentation currently mentions\nFRSX, if someone wants to write a patch for it.  (Such a change would\nsit in \"pu\" or \"next\" until after 2.0.0 is released, of course.)\n\nIn the meantime, when you're on machines using the current default,\nyou have two choices:\n\n a) set the LESS envvar in your .profile explicitly\n b) hit the two keys '-', shift+S when git opens a pager\n\nThe argument about safety is a red herring here, since it's always\npossible that a patch will wrap to make new lines with '+' or '-' or\n'@@' at the beginning that are equally confusing.\n\nHoping that clarifies,\nJonathan\n"},{"id":"239630","messageId":"vpqfvl1rj7i.fsf@anie.imag.fr","threadId":"36485","inReplyTo":"xmqqha5iv9eb.fsf@gitster.dls.corp.google.com","subject":"Re: Harmful LESS flags","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2014-04-25T06:56:01Z","receivedAt":"2014-04-25T06:56:01Z","isPatch":false,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> I am not opposed to changing the default in the longer term, as long\n> as we have a solid transition plan to ensure that it won't disrupt\n> and/or upset existing users too much.\n\nI am personally in favor of changing the default to drop the S. Silently\nhiding stuff from the user's eyes is really bad. With good coding\nstandard and reasonable terminal size, it actually doesn't matter. And\non projects actually containing very long lines (e.g. some people write\nLaTeX code with whole paragraphs for each lines), showing only the\nbeginning of the line (i.e. the first line of the paragraph in my\nLaTeX example) isn't very useful.\n\nI do not think we particularly need a transition plan here: it's purely\na user-interface thing, not something that may break any script or other\ntool. Changing the default and documenting the way to return to the old\ndefault in release notes and in the manual would be sufficient IMHO.\n\nGUI usually don't warn when the shape of a button is going to change in\nthe next version ...\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"239641","messageId":"20140425151124.GA11479@google.com","threadId":"36485","inReplyTo":"vpqfvl1rj7i.fsf@anie.imag.fr","subject":"Re: Harmful LESS flags","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2014-04-25T15:11:24Z","receivedAt":"2014-04-25T15:11:24Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nMatthieu Moy wrote:\n\n> I am personally in favor of changing the default to drop the S. Silently\n> hiding stuff from the user's eyes is really bad. With good coding\n> standard and reasonable terminal size, it actually doesn't matter.\n\nJust for clarity: no, when we are talking about well formatted code,\n-S is actually a way better interface.\n\nThat's because indentation matters and makes it easy to take in code\nstructure at a glance, long lines that get cut off by the margin stick\nout like a sore thumb already, and lines wrapped at an arbitrary\ncharacter are even more distracting to the point of being useless.\n\nIn practice I believe the \"Silently hiding stuff\" concern is much\nharder to solve.  In the case of malicious code that opened this\nthread, I think a marker on the right margin would reveal the\nwhitespace more clearly than wrapping that the reader may or may not\nnotice.\n\nLuckily, it is very easy to switch between the two views on the fly\n--- in an already-open \"less\" window, you just type '-' + 'S'.  In the\nspirit of not overriding tool defaults when there is not a strong\nreason to do so, I agree that if someone writes a patch to drop\nthe 'S' I would probably like it.\n\n[...]\n> I do not think we particularly need a transition plan here: it's purely\n> a user-interface thing, not something that may break any script or other\n> tool.\n\nAgreed ---- a note in release notes and making sure the documentation\nreflects the new default should be enough.\n\nThanks and hope that helps,\nJonathan\n"},{"id":"239642","messageId":"8761lxa0gs.fsf@fencepost.gnu.org","threadId":"36485","inReplyTo":"20140425151124.GA11479@google.com","subject":"Re: Harmful LESS flags","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2014-04-25T15:32:51Z","receivedAt":"2014-04-25T15:32:51Z","isPatch":false,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Jonathan Nieder <jrnieder@gmail.com> writes:\n\n> Matthieu Moy wrote:\n>\n>> I am personally in favor of changing the default to drop the S. Silently\n>> hiding stuff from the user's eyes is really bad. With good coding\n>> standard and reasonable terminal size, it actually doesn't matter.\n>\n> Just for clarity: no, when we are talking about well formatted code,\n> -S is actually a way better interface.\n\nWhen we are talking about well-formatted code, -S does not matter either\nwhich way.\n\n> That's because indentation matters and makes it easy to take in code\n> structure at a glance, long lines that get cut off by the margin stick\n> out like a sore thumb already, and lines wrapped at an arbitrary\n> character are even more distracting to the point of being useless.\n\nLines which are cut off are not \"to the point of being useless\", they\n_are_ useless.\n\nI am not arguing that wrapped lines are pretty.  And I also consider the\n\"malicious\" or \"hiding\" angle at best a marginal concern.\n\nOverriding less' defaults should only be done for unequivocal benefits,\nand in this case I consider the result actually more of a detriment than\nanything else.\n\n-- \nDavid Kastrup\n"},{"id":"239644","messageId":"20140425154722.GC11479@google.com","threadId":"36485","inReplyTo":"8761lxa0gs.fsf@fencepost.gnu.org","subject":"Re: Harmful LESS flags","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2014-04-25T15:47:22Z","receivedAt":"2014-04-25T15:47:22Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"David Kastrup wrote:\n> Jonathan Nieder <jrnieder@gmail.com> writes:\n\n>> Just for clarity: no, when we are talking about well formatted code,\n>> -S is actually a way better interface.\n>\n> When we are talking about well-formatted code, -S does not matter either\n> which way.\n\nSorry for the lack of clarity.  I believe well-formatted code can\ncontain long lines.  For example, sometimes a message + the printf to\nprint it and indentation move past the right margin.\n\nIf I wasn't talking about long lines, why would I have replied in the\nfirst place?\n\n[...]\n> Overriding less' defaults should only be done for unequivocal benefits,\n\nWe agree here.  So, does someone who actually wants this change want to\npropose a patch? :)\n\nHope that helps,\nJonathan\n"},{"id":"239839","messageId":"1398674062-24288-1-git-send-email-Matthieu.Moy@imag.fr","threadId":"36485","inReplyTo":"20140425154722.GC11479@google.com","subject":"[PATCH] PAGER_ENV: remove 'S' from $LESS by default","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2014-04-28T08:34:22Z","receivedAt":"2014-04-28T08:34:22Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"By default, Git used to set $LESS to -FRSX if $LESS was not set by the\nuser. The FRX flags actually make sense for Git (F and X because Git\nsometimes pipes short output to less, and R because Git pipes colored\noutput). The S flag (chop long lines), on the other hand, is not related\nto Git and is a matter of user preference. Git should not decide for the\nuser to change LESS's default.\n\nMore specifically, the S flag harms users who review untrusted code\nwithin a pager, since a patch looking like:\n\n-old code;\n+new good code; [... lots of tabs ...] malicious code;\n\nwould appear identical to:\n\n-old code;\n+new good code;\n\nUsers who prefer the old behavior can still set the $LESS environment\nvariable to -FRSX explicitly, or set core.pager to 'less -S'.\n\nThe documentation in config.txt is made a bit longer to keep both an\nexample setting the 'S' flag (needed to recover the old behavior) and an\nexample showing how to unset a flag set by Git.\n\nSigned-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n---\n> We agree here.  So, does someone who actually wants this change want to\n> propose a patch? :)\n\nHere you are.\n\n Documentation/config.txt | 13 ++++++++-----\n Makefile                 |  6 +++---\n perl/Git/SVN/Log.pm      |  2 +-\n 3 files changed, 12 insertions(+), 9 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex e30561d..b7f92ac 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -560,14 +560,17 @@ core.pager::\n \tconfiguration, then `$PAGER`, and then the default chosen at\n \tcompile time (usually 'less').\n +\n-When the `LESS` environment variable is unset, Git sets it to `FRSX`\n+When the `LESS` environment variable is unset, Git sets it to `FRX`\n (if `LESS` environment variable is set, Git does not change it at\n all).  If you want to selectively override Git's default setting\n-for `LESS`, you can set `core.pager` to e.g. `less -+S`.  This will\n+for `LESS`, you can set `core.pager` to e.g. `less -S`.  This will\n be passed to the shell by Git, which will translate the final\n-command to `LESS=FRSX less -+S`. The environment tells the command\n-to set the `S` option to chop long lines but the command line\n-resets it to the default to fold long lines.\n+command to `LESS=FRX less -S`. The environment does not set the\n+`S` option but the command line does, instructing less to truncate\n+long lines. Similarly, setting `core.pager` to `less -+F` will\n+deactivate the `F` option specified by the environment from the\n+command-line, deactivating the \"quit if one screen\" behavior of\n+`less`.\n +\n Likewise, when the `LV` environment variable is unset, Git sets it\n to `-c`.  You can override this setting by exporting `LV` with\ndiff --git a/Makefile b/Makefile\nindex a3b298e..cd3cdf6 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -344,9 +344,9 @@ all::\n # Define PAGER_ENV to a SP separated VAR=VAL pairs to define\n # default environment variables to be passed when a pager is spawned, e.g.\n #\n-#    PAGER_ENV = LESS=-FRSX LV=-c\n+#    PAGER_ENV = LESS=-FRX LV=-c\n #\n-# to say \"export LESS=-FRSX (and LV=-c) if the environment variable\n+# to say \"export LESS=-FRX (and LV=-c) if the environment variable\n # LESS (and LV) is not set, respectively\".\n \n GIT-VERSION-FILE: FORCE\n@@ -1518,7 +1518,7 @@ NO_PYTHON = NoThanks\n endif\n \n ifndef PAGER_ENV\n-PAGER_ENV = LESS=-FRSX LV=-c\n+PAGER_ENV = LESS=-FRX LV=-c\n endif\n \n QUIET_SUBDIR0  = +$(MAKE) -C # space to separate -C and subdir\ndiff --git a/perl/Git/SVN/Log.pm b/perl/Git/SVN/Log.pm\nindex 34f2869..6641053 100644\n--- a/perl/Git/SVN/Log.pm\n+++ b/perl/Git/SVN/Log.pm\n@@ -116,7 +116,7 @@ sub run_pager {\n \t\treturn;\n \t}\n \topen STDIN, '<&', $rfd or fatal \"Can't redirect stdin: $!\";\n-\t$ENV{LESS} ||= 'FRSX';\n+\t$ENV{LESS} ||= 'FRX';\n \t$ENV{LV} ||= '-c';\n \texec $pager or fatal \"Can't run pager: $! ($pager)\";\n }\n-- \n1.9.2.698.ge58c0c2.dirty\n"},{"id":"239842","messageId":"87vbtt6dyq.fsf@fencepost.gnu.org","threadId":"36485","inReplyTo":"1398674062-24288-1-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [PATCH] PAGER_ENV: remove 'S' from $LESS by default","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2014-04-28T08:43:57Z","receivedAt":"2014-04-28T08:43:57Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Matthieu Moy <Matthieu.Moy@imag.fr> writes:\n\n> By default, Git used to set $LESS to -FRSX if $LESS was not set by the\n> user. The FRX flags actually make sense for Git (F and X because Git\n> sometimes pipes short output to less, and R because Git pipes colored\n> output). The S flag (chop long lines), on the other hand, is not related\n> to Git and is a matter of user preference. Git should not decide for the\n> user to change LESS's default.\n\n>> We agree here.  So, does someone who actually wants this change want to\n>> propose a patch? :)\n>\n> Here you are.\n>\n>  Documentation/config.txt | 13 ++++++++-----\n>  Makefile                 |  6 +++---\n>  perl/Git/SVN/Log.pm      |  2 +-\n>  3 files changed, 12 insertions(+), 9 deletions(-)\n\nThere seem to be a few more occurences (git-sh-setup.sh and pager.c):\n\n$ git grep FRSX\nDocumentation/RelNotes/1.6.5.txt: * mingw will also give FRSX as the default val\nDocumentation/config.txt:When the `LESS` environment variable is unset, Git sets\nDocumentation/config.txt:command to `LESS=FRSX less -+S`. The environment tells \ngit-sh-setup.sh:        : ${LESS=-FRSX}\npager.c:                        env[i++] = \"LESS=FRSX\";\nperl/Git/SVN/Log.pm:    $ENV{LESS} ||= 'FRSX';\n\nSearching for LESS seems to implicate a few more possible candidates in\ncontrib/examples:\n\ncontrib/examples/git-log.sh:LESS=-S ${PAGER:-less}\ncontrib/examples/git-whatchanged.sh:LESS=\"$LESS -S\" ${PAGER:-less}\n\n\n-- \nDavid Kastrup\n"},{"id":"239846","messageId":"vpqsioxn82l.fsf@anie.imag.fr","threadId":"36485","inReplyTo":"87vbtt6dyq.fsf@fencepost.gnu.org","subject":"Re: [PATCH] PAGER_ENV: remove 'S' from $LESS by default","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2014-04-28T08:59:14Z","receivedAt":"2014-04-28T08:59:14Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"David Kastrup <dak@gnu.org> writes:\n\n> There seem to be a few more occurences (git-sh-setup.sh and pager.c):\n\nNot since f82c3ffd862c7 (Wed Feb 5 2014, move LESS/LV pager environment\nto Makefile).\n\n> Searching for LESS seems to implicate a few more possible candidates in\n> contrib/examples:\n>\n> contrib/examples/git-log.sh:LESS=-S ${PAGER:-less}\n> contrib/examples/git-whatchanged.sh:LESS=\"$LESS -S\" ${PAGER:-less}\n\nYes, I did see these, but I considered that contrib/examples/ should\nremain a snapshot of what the commands used to look like at the time\nthey were shell scripts.\n\nThere's also user-manual.txt:\n\n,----\n| Basically, the initial version of `git log` was a shell script:\n| \n| ----------------------------------------------------------------\n| $ git-rev-list --pretty $(git-rev-parse --default HEAD \"$@\") | \\\n| \tLESS=-S ${PAGER:-less}\n| ----------------------------------------------------------------\n`----\n\nthat I left intact. I can change them too if people prefer.\n\nThanks,\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"239901","messageId":"87k3a96cj9.fsf@fencepost.gnu.org","threadId":"36485","inReplyTo":"vpqsioxn82l.fsf@anie.imag.fr","subject":"Re: [PATCH] PAGER_ENV: remove 'S' from $LESS by default","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2014-04-28T09:14:50Z","receivedAt":"2014-04-28T09:14:50Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:\n\n> David Kastrup <dak@gnu.org> writes:\n>\n>> There seem to be a few more occurences (git-sh-setup.sh and pager.c):\n>\n> Not since f82c3ffd862c7 (Wed Feb 5 2014, move LESS/LV pager environment\n> to Makefile).\n\nThe only upstream branch containing this commit is pu.  So this patch\nshould likely not go anywhere else for now.\n\n-- \nDavid Kastrup\n"},{"id":"239904","messageId":"vpq38gxlk3m.fsf@anie.imag.fr","threadId":"36485","inReplyTo":"87k3a96cj9.fsf@fencepost.gnu.org","subject":"Re: [PATCH] PAGER_ENV: remove 'S' from $LESS by default","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2014-04-28T12:22:21Z","receivedAt":"2014-04-28T12:22:21Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"David Kastrup <dak@gnu.org> writes:\n\n> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:\n>\n>> David Kastrup <dak@gnu.org> writes:\n>>\n>>> There seem to be a few more occurences (git-sh-setup.sh and pager.c):\n>>\n>> Not since f82c3ffd862c7 (Wed Feb 5 2014, move LESS/LV pager environment\n>> to Makefile).\n>\n> The only upstream branch containing this commit is pu.  So this patch\n> should likely not go anywhere else for now.\n\nOops, indeed, I made my patch on top of pu by mistake. Anyway, my patch\ncan wait for the other series to be merged.\n\nJeff, you're the author of f82c3ffd862c7, topic jk/makefile in git.git,\nmarked \"expecting a reroll\" by Junio. Any news from the series?\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"239969","messageId":"20140428162439.GA9844@sigill.intra.peff.net","threadId":"36485","inReplyTo":"vpq38gxlk3m.fsf@anie.imag.fr","subject":"Re: [PATCH] PAGER_ENV: remove 'S' from $LESS by default","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-04-28T16:24:39Z","receivedAt":"2014-04-28T16:24:39Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Apr 28, 2014 at 02:22:21PM +0200, Matthieu Moy wrote:\n\n> >> Not since f82c3ffd862c7 (Wed Feb 5 2014, move LESS/LV pager environment\n> >> to Makefile).\n> >\n> > The only upstream branch containing this commit is pu.  So this patch\n> > should likely not go anywhere else for now.\n> \n> Oops, indeed, I made my patch on top of pu by mistake. Anyway, my patch\n> can wait for the other series to be merged.\n> \n> Jeff, you're the author of f82c3ffd862c7, topic jk/makefile in git.git,\n> marked \"expecting a reroll\" by Junio. Any news from the series?\n\nI am planning to revisit it eventually, but it's fairly low priority.\nThere is some pretty heavy refactoring in the series, and the PAGER_ENV\nbits do not have to be held hostage to that refactoring (they are really\njust demonstrating the refactoring).\n\nI'd be OK with doing the moral equivalent for now (perhaps just taking\nJunio's proposal[1]), and I can deal with the refactoring later when\nre-rolling the Makefile series.\n\n-Peff\n\n[1] http://article.gmane.org/gmane.comp.version-control.git/240637\n"},{"id":"239998","messageId":"xmqqk3a9qoib.fsf@gitster.dls.corp.google.com","threadId":"36485","inReplyTo":"20140428162439.GA9844@sigill.intra.peff.net","subject":"Re: [PATCH] PAGER_ENV: remove 'S' from $LESS by default","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-04-28T18:48:12Z","receivedAt":"2014-04-28T18:48:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Mon, Apr 28, 2014 at 02:22:21PM +0200, Matthieu Moy wrote:\n>\n> I'd be OK with doing the moral equivalent for now (perhaps just taking\n> Junio's proposal[1]), and I can deal with the refactoring later when\n> re-rolling the Makefile series.\n>\n> -Peff\n>\n> [1] http://article.gmane.org/gmane.comp.version-control.git/240637\n\nI doubt we would want to use the patch verbatim in that message; it\nserved its purpose well to illustrate that there may be other ways\nto address the issue, but I agreed with the flaw in it you pointed\nout in the thread [*1*]\n\nI also agree that droppage of S does not have to wait for that\ntopic.\n\n\n[Reference]\n*1* http://thread.gmane.org/gmane.comp.version-control.git/240548/focus=240746\n"},{"id":"240053","messageId":"535ECA3C.8090604@greenwoodsoftware.com","threadId":"36485","inReplyTo":"20140424001126.GG15516@google.com","subject":"Re: Harmful LESS flags","fromName":"Mark Nudelman","fromEmail":"markn@greenwoodsoftware.com","sentAt":"2014-04-28T21:38:04Z","receivedAt":"2014-04-28T21:38:04Z","isPatch":false,"sender":{"key":"markn@greenwoodsoftware.com","avatar":null},"body":"On 4/23/2014 5:11 PM, Jonathan Nieder wrote:\n> That sounds like a nice feature request for 'less': a marker on the\n> right margin when --chop-long-lines is in use and a line has been\n> chopped.  I don't see it at\n> http://www.greenwoodsoftware.com/less/bugs.html#enhance so maybe no\n> one else has thought of it yet.\n> \n> Mark, what do you think?\n> \n\nHi Jonathan,\nThis seems reasonable.  I actually thought that something like this was\nalready implemented, and displayed in the status column when -J is in\neffect.  But I must have dreamed that or something.  I'll add this as an\nenhancement request.\n\n--Mark\n"},{"id":"240173","messageId":"vpq4n1cxqrp.fsf@anie.imag.fr","threadId":"36485","inReplyTo":"xmqqk3a9qoib.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] PAGER_ENV: remove 'S' from $LESS by default","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2014-04-29T12:29:46Z","receivedAt":"2014-04-29T12:29:46Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Jeff King <peff@peff.net> writes:\n>\n>> On Mon, Apr 28, 2014 at 02:22:21PM +0200, Matthieu Moy wrote:\n>>\n>> I'd be OK with doing the moral equivalent for now (perhaps just taking\n>> Junio's proposal[1]), and I can deal with the refactoring later when\n>> re-rolling the Makefile series.\n>>\n>> -Peff\n>>\n>> [1] http://article.gmane.org/gmane.comp.version-control.git/240637\n>\n> I doubt we would want to use the patch verbatim in that message; it\n> served its purpose well to illustrate that there may be other ways\n> to address the issue, but I agreed with the flaw in it you pointed\n> out in the thread [*1*]\n>\n> I also agree that droppage of S does not have to wait for that\n> topic.\n\nSo, shall I rewrite my patch on top of master? (not hard, but there will\nbe a minor conflict to resolve when merging with Peff's cooking series).\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"240184","messageId":"xmqq38gwm5ny.fsf@gitster.dls.corp.google.com","threadId":"36485","inReplyTo":"vpq4n1cxqrp.fsf@anie.imag.fr","subject":"Re: [PATCH] PAGER_ENV: remove 'S' from $LESS by default","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-04-29T17:01:05Z","receivedAt":"2014-04-29T17:01:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:\n\n>> I also agree that droppage of S does not have to wait for that\n>> topic.\n>\n> So, shall I rewrite my patch on top of master? (not hard, but there will\n> be a minor conflict to resolve when merging with Peff's cooking series).\n\nSure, the one near the tip of 'pu' can even be dropped, especially\nwhen nobody is actively looking at it, if it turns out to be too\nmuch of a nuisance.\n\nThanks.\n"},{"id":"240274","messageId":"1398843325-31267-1-git-send-email-Matthieu.Moy@imag.fr","threadId":"36485","inReplyTo":"xmqq38gwm5ny.fsf@gitster.dls.corp.google.com","subject":"[PATCH v2] pager: remove 'S' from $LESS by default","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2014-04-30T07:35:25Z","receivedAt":"2014-04-30T07:35:25Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"By default, Git used to set $LESS to -FRSX if $LESS was not set by the\nuser. The FRX flags actually make sense for Git (F and X because Git\nsometimes pipes short output to less, and R because Git pipes colored\noutput). The S flag (chop long lines), on the other hand, is not related\nto Git and is a matter of user preference. Git should not decide for the\nuser to change LESS's default.\n\nMore specifically, the S flag harms users who review untrusted code\nwithin a pager, since a patch looking like:\n\n-old code;\n+new good code; [... lots of tabs ...] malicious code;\n\nwould appear identical to:\n\n-old code;\n+new good code;\n\nUsers who prefer the old behavior can still set the $LESS environment\nvariable to -FRSX explicitly, or set core.pager to 'less -S'.\n\nThe documentation in config.txt is made a bit longer to keep both an\nexample setting the 'S' flag (needed to recover the old behavior) and an\nexample showing how to unset a flag set by Git.\n\nSigned-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n---\nThis is just a rewrite of PATCH v1 on top of master instead of pu.\n\n Documentation/config.txt | 13 ++++++++-----\n git-sh-setup.sh          |  2 +-\n pager.c                  |  2 +-\n perl/Git/SVN/Log.pm      |  2 +-\n 4 files changed, 11 insertions(+), 8 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 73c8973..5484d9d 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -553,14 +553,17 @@ core.pager::\n \tconfiguration, then `$PAGER`, and then the default chosen at\n \tcompile time (usually 'less').\n +\n-When the `LESS` environment variable is unset, Git sets it to `FRSX`\n+When the `LESS` environment variable is unset, Git sets it to `FRX`\n (if `LESS` environment variable is set, Git does not change it at\n all).  If you want to selectively override Git's default setting\n-for `LESS`, you can set `core.pager` to e.g. `less -+S`.  This will\n+for `LESS`, you can set `core.pager` to e.g. `less -S`.  This will\n be passed to the shell by Git, which will translate the final\n-command to `LESS=FRSX less -+S`. The environment tells the command\n-to set the `S` option to chop long lines but the command line\n-resets it to the default to fold long lines.\n+command to `LESS=FRX less -S`. The environment does not set the\n+`S` option but the command line does, instructing less to truncate\n+long lines. Similarly, setting `core.pager` to `less -+F` will\n+deactivate the `F` option specified by the environment from the\n+command-line, deactivating the \"quit if one screen\" behavior of\n+`less`.\n +\n Likewise, when the `LV` environment variable is unset, Git sets it\n to `-c`.  You can override this setting by exporting `LV` with\ndiff --git a/git-sh-setup.sh b/git-sh-setup.sh\nindex 5f28b32..9447980 100644\n--- a/git-sh-setup.sh\n+++ b/git-sh-setup.sh\n@@ -160,7 +160,7 @@ git_pager() {\n \telse\n \t\tGIT_PAGER=cat\n \tfi\n-\t: ${LESS=-FRSX}\n+\t: ${LESS=-FRX}\n \t: ${LV=-c}\n \texport LESS LV\n \ndiff --git a/pager.c b/pager.c\nindex 0cc75a8..f75e8ae 100644\n--- a/pager.c\n+++ b/pager.c\n@@ -85,7 +85,7 @@ void setup_pager(void)\n \t\tint i = 0;\n \n \t\tif (!getenv(\"LESS\"))\n-\t\t\tenv[i++] = \"LESS=FRSX\";\n+\t\t\tenv[i++] = \"LESS=FRX\";\n \t\tif (!getenv(\"LV\"))\n \t\t\tenv[i++] = \"LV=-c\";\n \t\tenv[i] = NULL;\ndiff --git a/perl/Git/SVN/Log.pm b/perl/Git/SVN/Log.pm\nindex 34f2869..6641053 100644\n--- a/perl/Git/SVN/Log.pm\n+++ b/perl/Git/SVN/Log.pm\n@@ -116,7 +116,7 @@ sub run_pager {\n \t\treturn;\n \t}\n \topen STDIN, '<&', $rfd or fatal \"Can't redirect stdin: $!\";\n-\t$ENV{LESS} ||= 'FRSX';\n+\t$ENV{LESS} ||= 'FRX';\n \t$ENV{LV} ||= '-c';\n \texec $pager or fatal \"Can't run pager: $! ($pager)\";\n }\n-- \n1.9.2.698.ge58c0c2.dirty\n"},{"id":"240313","messageId":"xmqqbnvihlny.fsf@gitster.dls.corp.google.com","threadId":"36485","inReplyTo":"1398843325-31267-1-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [PATCH v2] pager: remove 'S' from $LESS by default","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-04-30T15:38:57Z","receivedAt":"2014-04-30T15:38:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy <Matthieu.Moy@imag.fr> writes:\n\n> By default, Git used to set $LESS to -FRSX if $LESS was not set by the\n> user. The FRX flags actually make sense for Git (F and X because Git\n> sometimes pipes short output to less, and R because Git pipes colored\n> output). The S flag (chop long lines), on the other hand, is not related\n> to Git and is a matter of user preference. Git should not decide for the\n> user to change LESS's default.\n\nGit always pipes its output to less, not just \"sometimes pipes short\nones\" ;-)  \"because the output may be short\" would be more accurate\nand would convey the same thing.\n\nBut that is just nitpicking.  The patch looks totally agreeable.\n\nI am inclined to suggest queuing it for the first batch after 2.0\ninstead of directly applying to 'master', as we have past the point\nwe can expect to see reports of unexpected fallouts and fix the\nissues in time for the final.\n\nThanks.\n\n> More specifically, the S flag harms users who review untrusted code\n> within a pager, since a patch looking like:\n>\n> -old code;\n> +new good code; [... lots of tabs ...] malicious code;\n>\n> would appear identical to:\n>\n> -old code;\n> +new good code;\n>\n> Users who prefer the old behavior can still set the $LESS environment\n> variable to -FRSX explicitly, or set core.pager to 'less -S'.\n>\n> The documentation in config.txt is made a bit longer to keep both an\n> example setting the 'S' flag (needed to recover the old behavior) and an\n> example showing how to unset a flag set by Git.\n>\n> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n> ---\n> This is just a rewrite of PATCH v1 on top of master instead of pu.\n>\n>  Documentation/config.txt | 13 ++++++++-----\n>  git-sh-setup.sh          |  2 +-\n>  pager.c                  |  2 +-\n>  perl/Git/SVN/Log.pm      |  2 +-\n>  4 files changed, 11 insertions(+), 8 deletions(-)\n>\n> diff --git a/Documentation/config.txt b/Documentation/config.txt\n> index 73c8973..5484d9d 100644\n> --- a/Documentation/config.txt\n> +++ b/Documentation/config.txt\n> @@ -553,14 +553,17 @@ core.pager::\n>  \tconfiguration, then `$PAGER`, and then the default chosen at\n>  \tcompile time (usually 'less').\n>  +\n> -When the `LESS` environment variable is unset, Git sets it to `FRSX`\n> +When the `LESS` environment variable is unset, Git sets it to `FRX`\n>  (if `LESS` environment variable is set, Git does not change it at\n>  all).  If you want to selectively override Git's default setting\n> -for `LESS`, you can set `core.pager` to e.g. `less -+S`.  This will\n> +for `LESS`, you can set `core.pager` to e.g. `less -S`.  This will\n>  be passed to the shell by Git, which will translate the final\n> -command to `LESS=FRSX less -+S`. The environment tells the command\n> -to set the `S` option to chop long lines but the command line\n> -resets it to the default to fold long lines.\n> +command to `LESS=FRX less -S`. The environment does not set the\n> +`S` option but the command line does, instructing less to truncate\n> +long lines. Similarly, setting `core.pager` to `less -+F` will\n> +deactivate the `F` option specified by the environment from the\n> +command-line, deactivating the \"quit if one screen\" behavior of\n> +`less`.\n>  +\n>  Likewise, when the `LV` environment variable is unset, Git sets it\n>  to `-c`.  You can override this setting by exporting `LV` with\n> diff --git a/git-sh-setup.sh b/git-sh-setup.sh\n> index 5f28b32..9447980 100644\n> --- a/git-sh-setup.sh\n> +++ b/git-sh-setup.sh\n> @@ -160,7 +160,7 @@ git_pager() {\n>  \telse\n>  \t\tGIT_PAGER=cat\n>  \tfi\n> -\t: ${LESS=-FRSX}\n> +\t: ${LESS=-FRX}\n>  \t: ${LV=-c}\n>  \texport LESS LV\n>  \n> diff --git a/pager.c b/pager.c\n> index 0cc75a8..f75e8ae 100644\n> --- a/pager.c\n> +++ b/pager.c\n> @@ -85,7 +85,7 @@ void setup_pager(void)\n>  \t\tint i = 0;\n>  \n>  \t\tif (!getenv(\"LESS\"))\n> -\t\t\tenv[i++] = \"LESS=FRSX\";\n> +\t\t\tenv[i++] = \"LESS=FRX\";\n>  \t\tif (!getenv(\"LV\"))\n>  \t\t\tenv[i++] = \"LV=-c\";\n>  \t\tenv[i] = NULL;\n> diff --git a/perl/Git/SVN/Log.pm b/perl/Git/SVN/Log.pm\n> index 34f2869..6641053 100644\n> --- a/perl/Git/SVN/Log.pm\n> +++ b/perl/Git/SVN/Log.pm\n> @@ -116,7 +116,7 @@ sub run_pager {\n>  \t\treturn;\n>  \t}\n>  \topen STDIN, '<&', $rfd or fatal \"Can't redirect stdin: $!\";\n> -\t$ENV{LESS} ||= 'FRSX';\n> +\t$ENV{LESS} ||= 'FRX';\n>  \t$ENV{LV} ||= '-c';\n>  \texec $pager or fatal \"Can't run pager: $! ($pager)\";\n>  }\n"},{"id":"240314","messageId":"vpqy4ymlsvy.fsf@anie.imag.fr","threadId":"36485","inReplyTo":"xmqqbnvihlny.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2] pager: remove 'S' from $LESS by default","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2014-04-30T15:49:21Z","receivedAt":"2014-04-30T15:49:21Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Matthieu Moy <Matthieu.Moy@imag.fr> writes:\n>\n>> By default, Git used to set $LESS to -FRSX if $LESS was not set by the\n>> user. The FRX flags actually make sense for Git (F and X because Git\n>> sometimes pipes short output to less, and R because Git pipes colored\n>> output). The S flag (chop long lines), on the other hand, is not related\n>> to Git and is a matter of user preference. Git should not decide for the\n>> user to change LESS's default.\n>\n> Git always pipes its output to less,\n\nErr, no, not all commands use a pager.\n\n> I am inclined to suggest queuing it for the first batch after 2.0\n> instead of directly applying to 'master', as we have past the point\n> we can expect to see reports of unexpected fallouts and fix the\n> issues in time for the final.\n\nFine with me.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"240339","messageId":"xmqqvbtqg1ri.fsf@gitster.dls.corp.google.com","threadId":"36485","inReplyTo":"vpqy4ymlsvy.fsf@anie.imag.fr","subject":"Re: [PATCH v2] pager: remove 'S' from $LESS by default","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-04-30T17:34:09Z","receivedAt":"2014-04-30T17:34:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:\n\n> Junio C Hamano <gitster@pobox.com> writes:\n>\n>> Matthieu Moy <Matthieu.Moy@imag.fr> writes:\n>>\n>>> By default, Git used to set $LESS to -FRSX if $LESS was not set by the\n>>> user. The FRX flags actually make sense for Git (F and X because Git\n>>> sometimes pipes short output to less, and R because Git pipes colored\n>>> output). The S flag (chop long lines), on the other hand, is not related\n>>> to Git and is a matter of user preference. Git should not decide for the\n>>> user to change LESS's default.\n>>\n>> Git always pipes its output to less,\n>\n> Err, no, not all commands use a pager.\n\nYeah, what I meant to say was that when it is told to use pager\n(either by an explicit \"git -p\" or implicitly with the lack of \"git\n--no-pager\"), it does not count the lines shown to avoid passing\nshort output to the pager.  \"sometimes pipes\" sounded to me as if we\nattempt to do so and sometimes fail to.  But again, as I said, that\nis just nitpicking.\n\n>> I am inclined to suggest queuing it for the first batch after 2.0\n>> instead of directly applying to 'master', as we have past the point\n>> we can expect to see reports of unexpected fallouts and fix the\n>> issues in time for the final.\n>\n> Fine with me.\n"},{"id":"240749","messageId":"20140505184441.GS9218@google.com","threadId":"36485","inReplyTo":"1398843325-31267-1-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [PATCH v2] pager: remove 'S' from $LESS by default","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2014-05-05T18:44:41Z","receivedAt":"2014-05-05T18:44:41Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nMatthieu Moy wrote:\n\n> By default, Git used to set $LESS to -FRSX if $LESS was not set by the\n> user. The FRX flags actually make sense for Git (F and X because Git\n> sometimes pipes short output to less, and R because Git pipes colored\n> output). The S flag (chop long lines), on the other hand, is not related\n> to Git and is a matter of user preference. Git should not decide for the\n> user to change LESS's default.\n\nThanks!  Sounds like a very good change.\n\n(Nit: instead of \"because Git sometimes pipes short output to less\",\nit would be clearer to say something like \"when Git pipes short output\nto less it is nice to exit and let the user type their next command\".)\n\n[...]\n> The documentation in config.txt is made a bit longer to keep both an\n> example setting the 'S' flag (needed to recover the old behavior) and an\n> example showing how to unset a flag set by Git.\n\nInteresting.  Looks good.\n\nFor what it's worth,\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\n"},{"id":"240730","messageId":"110110563.544859.1399320654149.JavaMail.zimbra@imag.fr","threadId":"36485","inReplyTo":"20140505184441.GS9218@google.com","subject":"Re: [PATCH v2] pager: remove 'S' from $LESS by default","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2014-05-05T20:10:54Z","receivedAt":"2014-05-05T20:10:54Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"----- Original Message -----\n> Hi,\n> \n> Matthieu Moy wrote:\n> \n> > By default, Git used to set $LESS to -FRSX if $LESS was not set by the\n> > user. The FRX flags actually make sense for Git (F and X because Git\n> > sometimes pipes short output to less, and R because Git pipes colored\n> > output). The S flag (chop long lines), on the other hand, is not related\n> > to Git and is a matter of user preference. Git should not decide for the\n> > user to change LESS's default.\n> \n> Thanks!  Sounds like a very good change.\n> \n> (Nit: instead of \"because Git sometimes pipes short output to less\",\n> it would be clearer to say something like \"when Git pipes short output\n> to less it is nice to exit and let the user type their next command\".)\n\nIt's actually a bit more than this: X to avoid initializing the terminal\nand F for the exit behavior you describe.\n\nBut since the change is actually not about F and X, I prefered keeping\nthe text about them as short as possible, so I prefer my version actually.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"240810","messageId":"xmqqppjqg6an.fsf@gitster.dls.corp.google.com","threadId":"36485","inReplyTo":"110110563.544859.1399320654149.JavaMail.zimbra@imag.fr","subject":"Re: [PATCH v2] pager: remove 'S' from $LESS by default","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-05-06T17:34:24Z","receivedAt":"2014-05-06T17:34:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy <matthieu.moy@grenoble-inp.fr> writes:\n\n>> > By default, Git used to set $LESS to -FRSX if $LESS was not set by the\n>> > user. The FRX flags actually make sense for Git (F and X because Git\n>> > sometimes pipes short output to less, and R because Git pipes colored\n>> > output). The S flag (chop long lines), on the other hand, is not related\n>> > to Git and is a matter of user preference. Git should not decide for the\n>> > user to change LESS's default.\n>> \n>> Thanks!  Sounds like a very good change.\n>> \n>> (Nit: instead of \"because Git sometimes pipes short output to less\",\n>> it would be clearer to say something like \"when Git pipes short output\n>> to less it is nice to exit and let the user type their next command\".)\n>\n> It's actually a bit more than this: X to avoid initializing the terminal\n> and F for the exit behavior you describe.\n>\n> But since the change is actually not about F and X, I prefered keeping\n> the text about them as short as possible, so I prefer my version actually.\n\nTrue.\n\nAs some of you might know, the version I use for my regular work is\nslightly ahead of 'next' (you can see where it is by running \"git\nlog --oneline --first-parent master..pu\" and find the first entry\nmarked as \"Merge ... into jch\").  After having this patch for a few\ndays in there and using it, I have to say that I like this change a\nlot while viewing the \"git log -p\" output.\n\nI still find the output from \"git blame\" disturbing, though.  The\nfirst thing I do in \"git blame\" output is to scroll to the right in\norder to identify the the area I am interested in, and this first\nstep is not negatively affected, because the right scrolled output \nautomatically wraps long lines.\n\nBut my second step is to scroll back to the left edge to find the\ncommit object name and at that point, the new default output without\n\"S\" gets somewhat annoying, because most of the output lines from\n\"git blame\" are longer than my window width.\n"},{"id":"240814","messageId":"87mweuss7d.fsf@fencepost.gnu.org","threadId":"36485","inReplyTo":"xmqqppjqg6an.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2] pager: remove 'S' from $LESS by default","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2014-05-06T18:00:22Z","receivedAt":"2014-05-06T18:00:22Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> I still find the output from \"git blame\" disturbing, though.  The\n> first thing I do in \"git blame\" output is to scroll to the right in\n> order to identify the the area I am interested in, and this first\n> step is not negatively affected, because the right scrolled output \n> automatically wraps long lines.\n>\n> But my second step is to scroll back to the left edge to find the\n> commit object name and at that point, the new default output without\n> \"S\" gets somewhat annoying, because most of the output lines from\n> \"git blame\" are longer than my window width.\n\ngit blame sucks in anything but fullscreen either way.  It would help to\ndisplay _only_ the source code and have the other info as mouse-over,\nbut that's not something a pager can do.\n\nIt is a pity that the content can be columnized much worse than the\nmetadata: otherwise it would make much more sense to display the content\n_first_ in line.  The metadata is useless without the content anyway.\n\n-- \nDavid Kastrup\n"},{"id":"240824","messageId":"vpqzjiuu4i5.fsf@anie.imag.fr","threadId":"36485","inReplyTo":"87mweuss7d.fsf@fencepost.gnu.org","subject":"Re: [PATCH v2] pager: remove 'S' from $LESS by default","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2014-05-06T18:49:22Z","receivedAt":"2014-05-06T18:49:22Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"David Kastrup <dak@gnu.org> writes:\n\n> Junio C Hamano <gitster@pobox.com> writes:\n>\n>> I still find the output from \"git blame\" disturbing, though.  The\n>> first thing I do in \"git blame\" output is to scroll to the right in\n>> order to identify the the area I am interested in, and this first\n>> step is not negatively affected, because the right scrolled output \n>> automatically wraps long lines.\n>>\n>> But my second step is to scroll back to the left edge to find the\n>> commit object name and at that point, the new default output without\n>> \"S\" gets somewhat annoying, because most of the output lines from\n>> \"git blame\" are longer than my window width.\n>\n> git blame sucks in anything but fullscreen either way.  It would help to\n> display _only_ the source code and have the other info as mouse-over,\n> but that's not something a pager can do.\n\nExactly. I personally never use \"git blame\" outside \"git gui blame\" for\nthis reason.\n\nIt's possible for a user to set pager.blame to \"less -S\" to get back to\nthe previous behavior only for blame.\n\nThe idea of having a separate default value for pager.blame (or set\n$LESS differently for blame) crossed my mind, but I actually don't like\nit, as it would make it harder for a user to fine-tune his configuration\nmanually (one would have to cancel all the corner-cases that Git would\nset by default).\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"240843","messageId":"20140506215516.GA30185@sigill.intra.peff.net","threadId":"36485","inReplyTo":"vpqzjiuu4i5.fsf@anie.imag.fr","subject":"Re: [PATCH v2] pager: remove 'S' from $LESS by default","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-05-06T21:55:16Z","receivedAt":"2014-05-06T21:55:16Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, May 06, 2014 at 08:49:22PM +0200, Matthieu Moy wrote:\n\n> Exactly. I personally never use \"git blame\" outside \"git gui blame\" for\n> this reason.\n\nI'd recommend \"tig blame\" for this, too, which behaves like \"less -S\"\nwith respect to long lines (and also makes it easy to jump to the full\ndiff, or restart the blame from the parent of the found commit).\n\n> It's possible for a user to set pager.blame to \"less -S\" to get back to\n> the previous behavior only for blame.\n> \n> The idea of having a separate default value for pager.blame (or set\n> $LESS differently for blame) crossed my mind, but I actually don't like\n> it, as it would make it harder for a user to fine-tune his configuration\n> manually (one would have to cancel all the corner-cases that Git would\n> set by default).\n\nAgreed. We already get some confusion from users with \"git has set $LESS\nfor me\".  Changing it to \"git set up $LESS depending on which command is\nrunning\" seems like it would cause more of the same.\n\n-Peff\n"},{"id":"240919","messageId":"xmqqy4ydbjqm.fsf@gitster.dls.corp.google.com","threadId":"36485","inReplyTo":"20140506215516.GA30185@sigill.intra.peff.net","subject":"Re: [PATCH v2] pager: remove 'S' from $LESS by default","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-05-07T17:07:29Z","receivedAt":"2014-05-07T17:07:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Tue, May 06, 2014 at 08:49:22PM +0200, Matthieu Moy wrote:\n> ...\n>> The idea of having a separate default value for pager.blame (or set\n>> $LESS differently for blame) crossed my mind, but I actually don't like\n>> it, as it would make it harder for a user to fine-tune his configuration\n>> manually (one would have to cancel all the corner-cases that Git would\n>> set by default).\n>\n> Agreed. We already get some confusion from users with \"git has set $LESS\n> for me\".  Changing it to \"git set up $LESS depending on which command is\n> running\" seems like it would cause more of the same.\n\nWhile I fully agree with the above conclusion, I just noticed that I\nwill be irritated enough to eventually set pager.blame myself, after\nrunning a short \"git blame -L1311,+7 git-p4.py\", which is one of the\nstandard first steps for me to start reading patches submit on the\nlist.\n\nEven with line wrapping, the output fits on a single page, so \"F\"\n(aka \"--quit-if-one-screen\") kicks in, before giving me a chance to\nscroll horizontally around.\n\nNot objecting to the conclusion of the discussion with a concrete\ncounterproposal.  Just hoping somebody clever enough might come up\nwith a good trick to make things easier to use and explain, which I\nhowever suspect may be an incompatible pair of goals.\n"},{"id":"240924","messageId":"vpqlhudqxto.fsf@anie.imag.fr","threadId":"36485","inReplyTo":"xmqqy4ydbjqm.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2] pager: remove 'S' from $LESS by default","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2014-05-07T17:54:11Z","receivedAt":"2014-05-07T17:54:11Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> While I fully agree with the above conclusion, I just noticed that I\n> will be irritated enough to eventually set pager.blame myself, after\n> running a short \"git blame -L1311,+7 git-p4.py\", which is one of the\n> standard first steps for me to start reading patches submit on the\n> list.\n\nPerhaps it deserves a mention in the doc, e.g. squashing this on top of\nmy patch:\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex b7f92ac..ebd1676 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -570,7 +570,9 @@ command to `LESS=FRX less -S`. The environment does not set the\n long lines. Similarly, setting `core.pager` to `less -+F` will\n deactivate the `F` option specified by the environment from the\n command-line, deactivating the \"quit if one screen\" behavior of\n-`less`.\n+`less`.  One can specifically activate some flags for particular\n+commands: for example, setting `pager.blame` to `less -S` enables\n+line truncation only for `git blame`.\n +\n Likewise, when the `LV` environment variable is unset, Git sets it\n to `-c`.  You can override this setting by exporting `LV` with\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"240951","messageId":"xmqqioph8go5.fsf@gitster.dls.corp.google.com","threadId":"36485","inReplyTo":"vpqlhudqxto.fsf@anie.imag.fr","subject":"Re: [PATCH v2] pager: remove 'S' from $LESS by default","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-05-07T20:42:02Z","receivedAt":"2014-05-07T20:42:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:\n\n> Perhaps it deserves a mention in the doc, e.g. squashing this on top of\n> my patch:\n\nThanks, will do.\n\n>\n> diff --git a/Documentation/config.txt b/Documentation/config.txt\n> index b7f92ac..ebd1676 100644\n> --- a/Documentation/config.txt\n> +++ b/Documentation/config.txt\n> @@ -570,7 +570,9 @@ command to `LESS=FRX less -S`. The environment does not set the\n>  long lines. Similarly, setting `core.pager` to `less -+F` will\n>  deactivate the `F` option specified by the environment from the\n>  command-line, deactivating the \"quit if one screen\" behavior of\n> -`less`.\n> +`less`.  One can specifically activate some flags for particular\n> +commands: for example, setting `pager.blame` to `less -S` enables\n> +line truncation only for `git blame`.\n>  +\n>  Likewise, when the `LV` environment variable is unset, Git sets it\n>  to `-c`.  You can override this setting by exporting `LV` with\n"}]}