{"thread":{"id":"15159","subject":"[PATCH RESEND] Do not override LESS","startedAt":"2008-08-22T12:25:12Z","lastAt":"2008-08-24T18:59:30Z","messageCount":8,"participants":["Anders Melchiorsen","Junio C Hamano","Jonathan Nieder"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"88119","messageId":"1219407912-32085-1-git-send-email-mail@cup.kalibalik.dk","threadId":"15159","inReplyTo":null,"subject":"[PATCH RESEND] Do not override LESS","fromName":"Anders Melchiorsen","fromEmail":"mail@cup.kalibalik.dk","sentAt":"2008-08-22T12:25:12Z","receivedAt":"2008-08-22T12:25:12Z","isPatch":true,"sender":{"key":"mail@cup.kalibalik.dk","avatar":null},"body":"Passing options to \"less\" with the LESS environment variable can\ninterfere with existing environment variables. There are at least two\nproblems, as the following examples show:\n\n1. Alice is using git with colors. Now she decides to set LESS=i for\nsome reason. Suddenly, she sees codes in place of colors because LESS\nis no longer set automatically.\n\n2. Bob sets GIT_PAGER=\"less -RS\", but does not set LESS. Git sets\nLESS=FRSX before calling $GIT_PAGER. Now Bob wonders why his pager is\nnot always paging, when he explicitly tried to clear the F option.\n\nBy passing the options on the command line instead, both of these\nsituations are handled.\n\nSigned-off-by: Anders Melchiorsen \n---\n\n\nHere is a resend, as I got no comments on the first try. The patch\nis rebased to the recent pager-swap setup.\n\nFor completeness, I note that this change will make existing setups\nwith PAGER=\"less\" behave differently, as they will no longer have\n-FRSX options set automatically. I tend to think that this is correct,\nsince you then get what you ask for.\n\n\n pager.c | 4 +---\n 1 files changed, 1 insertions(+), 3 deletions(-)\n\ndiff --git a/pager.c b/pager.c\nindex aa0966c..9753fe9 100644\n--- a/pager.c\n+++ b/pager.c\n@@ -20,8 +20,6 @@ static void pager_preexec(void)\n \tFD_ZERO(&in);\n \tFD_SET(0, &in);\n \tselect(1, &in, NULL, &in, NULL);\n-\n-\tsetenv(\"LESS\", \"FRSX\", 0);\n }\n #endif\n \n@@ -52,7 +50,7 @@ void setup_pager(void)\n \tif (!pager)\n \t\tpager = getenv(\"PAGER\");\n \tif (!pager)\n-\t\tpager = \"less\";\n+\t\tpager = \"less -FRSX\";\n \telse if (!*pager || !strcmp(pager, \"cat\"))\n \t\treturn;\n \n-- \n1.5.6.4\n"},{"id":"88257","messageId":"7vvdxs2t03.fsf@gitster.siamese.dyndns.org","threadId":"15159","inReplyTo":"1219407912-32085-1-git-send-email-mail@cup.kalibalik.dk","subject":"Re: [PATCH RESEND] Do not override LESS","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-08-23T05:35:40Z","receivedAt":"2008-08-23T05:35:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Anders Melchiorsen writes:\n\n> Passing options to \"less\" with the LESS environment variable can\n> interfere with existing environment variables. There are at least two\n> problems, as the following examples show:\n>\n> 1. Alice is using git with colors. Now she decides to set LESS=i for\n> some reason. Suddenly, she sees codes in place of colors because LESS\n> is no longer set automatically.\n>\n> 2. Bob sets GIT_PAGER=\"less -RS\", but does not set LESS. Git sets\n> LESS=FRSX before calling $GIT_PAGER. Now Bob wonders why his pager is\n> not always paging, when he explicitly tried to clear the F option.\n\n3. Christ has been happily using git with his PAGER set to \"less\". He\n suddenly notices that output from git linewraps and the pager does not\n exit when showing a short output, and gets very unhappy.\n"},{"id":"88266","messageId":"87k5e8i18c.fsf@cup.kalibalik.dk","threadId":"15159","inReplyTo":"7vvdxs2t03.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH RESEND] Do not override LESS","fromName":"Anders Melchiorsen","fromEmail":"mail@cup.kalibalik.dk","sentAt":"2008-08-23T08:28:51Z","receivedAt":"2008-08-23T08:28:51Z","isPatch":true,"sender":{"key":"mail@cup.kalibalik.dk","avatar":null},"body":"Junio C Hamano writes:\n\n> Anders Melchiorsen writes:\n>\n>> Passing options to \"less\" with the LESS environment variable can\n>> interfere with existing environment variables. There are at least two\n>> problems, as the following examples show:\n>>\n>> 1. Alice is using git with colors. Now she decides to set LESS=i for\n>> some reason. Suddenly, she sees codes in place of colors because LESS\n>> is no longer set automatically.\n>>\n>> 2. Bob sets GIT_PAGER=\"less -RS\", but does not set LESS. Git sets\n>> LESS=FRSX before calling $GIT_PAGER. Now Bob wonders why his pager is\n>> not always paging, when he explicitly tried to clear the F option.\n>\n> 3. Christ has been happily using git with his PAGER set to \"less\". He\n> suddenly notices that output from git linewraps and the pager does not\n> exit when showing a short output, and gets very unhappy.\n\nWell, I noted that point already, so I had hoped for a reply\nexplaining why it is a big problem. Maybe setting PAGER=\"less\" is more\ncommon than I think, as I have never seen it.\n\nWhile I am wary of advocating a patch that makes Christ unhappy, the\n\"3.\" issue is easily fixed by him setting GIT_PAGER=\"less -FRSX\".\n\nMy concern is that without reading the source, it can be confusing to\nfigure out what happens with less, $LESS and git. I think my patch\nimproves on that. On the other hand, predictability is not really\nneeded if the current setup DWIM. And maybe it does.\n\nThis is hardly a big issue either way, so now that I have response on\nthe patch, I will not pursue it further.\n\n\nThanks,\nAnders.\n"},{"id":"88279","messageId":"7vtzdc14k5.fsf@gitster.siamese.dyndns.org","threadId":"15159","inReplyTo":"87k5e8i18c.fsf@cup.kalibalik.dk","subject":"Re: [PATCH RESEND] Do not override LESS","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-08-23T09:08:58Z","receivedAt":"2008-08-23T09:08:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Anders Melchiorsen writes:\n\n> Junio C Hamano writes:\n>\n>> 3. Christ has been happily using git with his PAGER set to \"less\". He\n>> suddenly notices that output from git linewraps and the pager does not\n>> exit when showing a short output, and gets very unhappy.\n>\n> Well, I noted that point already, so I had hoped for a reply\n> explaining why it is a big problem.\n\nIt is a huge problem. It breaks people's existing perfectly well working\nsetup.\n\nAnd I do not think it is impossible to solve the issue without doing so.\n\n> While I am wary of advocating a patch that makes Christ unhappy, the\n> \"3.\" issue is easily fixed by him setting GIT_PAGER=\"less -FRSX\".\n\nThat's not a solution. Alice and Bob can also change their environment to\ntheir taste as well. Why punish existing users?\n\nIf the problem you are trying to solve is that there is no existing\ncombination of the environment variables for them to do so, you can solve\nit by introducing a new configuration or environment to support such usage\nand documenting it, can't you?\n"},{"id":"88287","messageId":"8763pshw10.fsf@cup.kalibalik.dk","threadId":"15159","inReplyTo":"7vtzdc14k5.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH RESEND] Do not override LESS","fromName":"Anders Melchiorsen","fromEmail":"mail@cup.kalibalik.dk","sentAt":"2008-08-23T10:21:15Z","receivedAt":"2008-08-23T10:21:15Z","isPatch":true,"sender":{"key":"mail@cup.kalibalik.dk","avatar":null},"body":"Junio C Hamano writes:\n\n> That's not a solution. Alice and Bob can also change their\n> environment to their taste as well. Why punish existing users?\n\nAs I said, when you ask for \"less\", I tend to think it is correct to\nget \"less\". Not \"LESS=FSRX less\", with a special case if LESS is\nalready defined.\n\n> If the problem you are trying to solve is that there is no existing\n> combination of the environment variables for them to do so, you can\n> solve it by introducing a new configuration or environment to\n> support such usage and documenting it, can't you?\n\nI was trying to clean up things, not to make them even more confusing.\nIf backwards compatibility is more important to you, I can understand.\n\n\nCheers,\nAnders.\n"},{"id":"88350","messageId":"Pine.GSO.4.62.0808240019050.28567@harper.uchicago.edu","threadId":"15159","inReplyTo":"87k5e8i18c.fsf@cup.kalibalik.dk","subject":"Re: [PATCH RESEND] Do not override LESS","fromName":"Jonathan Nieder","fromEmail":"jrnieder@uchicago.edu","sentAt":"2008-08-24T05:28:32Z","receivedAt":"2008-08-24T05:28:32Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nAnders Melchiorsen wrote:\n\n> Junio C Hamano writes:\n> \n> > Anders Melchiorsen writes:\n> >\n> >> Passing options to \"less\" with the LESS environment variable can\n> >> interfere with existing environment variables.\n\n[...]\n> >> 2. Bob sets GIT_PAGER=\"less -RS\", but does not set LESS. Git sets\n> >> LESS=FRSX before calling $GIT_PAGER. Now Bob wonders why his pager is\n> >> not always paging, when he explicitly tried to clear the F option.\n> >\n> > 3. Christ has been happily using git with his PAGER set to \"less\". He\n> > suddenly notices that output from git linewraps and the pager does not\n> > exit when showing a short output, and gets very unhappy.\n> \n> Well, I noted that point already, so I had hoped for a reply\n> explaining why it is a big problem. Maybe setting PAGER=\"less\" is more\n> common than I think, as I have never seen it.\n\nOn system where less is not the default pager (e.g. Solaris), it is\nvery common.\n \n> While I am wary of advocating a patch that makes Christ unhappy, the\n> \"3.\" issue is easily fixed by him setting GIT_PAGER=\"less -FRSX\".\n> \n> My concern is that without reading the source, it can be confusing to\n> figure out what happens with less, $LESS and git.\n\nSo perhaps it is a documentation problem. How about this patch?\n\n-- %< --\nSubject: Documentation: clarify pager configuration\n\nThe unwary user may not know how to disable the -FRSX options.\n\nSigned-off-by: Jonathan Nieder \n---\n Documentation/config.txt | 9 +++++++--\n Documentation/git.txt | 3 ++-\n 2 files changed, 9 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 9020675..88638f7 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -358,8 +358,13 @@ core.editor::\n \t`EDITOR` environment variables and then finally `vi`.\n \n core.pager::\n-\tThe command that git will use to paginate output. Can be overridden\n-\twith the `GIT_PAGER` environment variable.\n+\tThe command that git will use to paginate output. Can\n+\tbe overridden with the `GIT_PAGER` environment\n+\tvariable. Note that git sets the `LESS` environment\n+\tvariable to `FRSX` if it is unset when it runs the\n+\tpager. One can change these settings by setting the\n+\t`LESS` variable to some other value or by giving the\n+\t`core.pager` option a value such as \"`less -+FRSX`\".\n \n core.whitespace::\n \tA comma separated list of common whitespace problems to\ndiff --git a/Documentation/git.txt b/Documentation/git.txt\nindex 1bc295d..363a785 100644\n--- a/Documentation/git.txt\n+++ b/Documentation/git.txt\n@@ -497,7 +497,8 @@ other\n 'GIT_PAGER'::\n \tThis environment variable overrides `$PAGER`. If it is set\n \tto an empty string or to the value \"cat\", git will not launch\n-\ta pager.\n+\ta pager. See also the `core.pager` option in\n+\tlinkgit:git-config[1].\n \n 'GIT_SSH'::\n \tIf this environment variable is set then 'git-fetch'\n-- \n1.6.0.481.g9ef3\n"},{"id":"88351","messageId":"Pine.GSO.4.62.0808240028450.28567@harper.uchicago.edu","threadId":"15159","inReplyTo":"Pine.GSO.4.62.0808240019050.28567@harper.uchicago.edu","subject":"[PATCH] Documentation: clarify pager. configuration","fromName":"Jonathan Nieder","fromEmail":"jrnieder@uchicago.edu","sentAt":"2008-08-24T05:38:06Z","receivedAt":"2008-08-24T05:38:06Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"It was not obvious from the text that pager. is a boolean\nsetting.\n\nWhile we're changing the description, make some other\nimprovements: lest we forget and fret, clarify that -p and\npager. do not kick in when stdout is not a tty; point to\nrelated core.pager and GIT_PAGER settings; use renamed --paginate\noption.\n\nSigned-off-by: Jonathan Nieder \n---\n This is not related to the core.pager documentation patch I just\n sent; it just caught my eye as I was reading over the file.\n\n Documentation/config.txt | 8 +++++---\n 1 files changed, 5 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 88638f7..b12e695 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -988,9 +988,11 @@ pack.packSizeLimit::\n \tlinkgit:git-repack[1].\n \n pager.::\n-\tAllows to set your own pager preferences for each command, overriding\n-\tthe default. If `\\--pager` or `\\--no-pager` is specified on the command\n-\tline, it takes precedence over this option.\n+\tAllows turning on or off pagination of the output of a\n+\tparticular git subcommand when outputing to a tty. If\n+\t`\\--paginate` or `\\--no-pager` is specified on the command line,\n+\tit takes precedence over this option. To disable pagination for\n+\tall commands, set `core.pager` or 'GIT_PAGER' to \"`cat`\".\n \n pull.octopus::\n \tThe default merge strategy to use when pulling multiple branches\n-- \n1.6.0.481.g9ef3\n"},{"id":"88381","messageId":"7vsksume7h.fsf@gitster.siamese.dyndns.org","threadId":"15159","inReplyTo":"Pine.GSO.4.62.0808240019050.28567@harper.uchicago.edu","subject":"Re: [PATCH RESEND] Do not override LESS","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-08-24T18:59:30Z","receivedAt":"2008-08-24T18:59:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> Anders Melchiorsen wrote:\n> ...\n>> My concern is that without reading the source, it can be confusing to\n>> figure out what happens with less, $LESS and git.\n>\n> So perhaps it is a documentation problem. How about this patch?\n\nI think this makes sense. Anders?\n"}]}