{"thread":{"id":"17023","subject":"[PATCH (topgit) 2/2] tg-patch: fix pagination","startedAt":"2009-01-06T15:16:37Z","lastAt":"2009-01-18T15:04:20Z","messageCount":14,"participants":["Kirill Smelkov","martin f krafft","Adeodato Simó","Thomas Rast","Bert Wesarg","Pierre Habouzit","Petr Baudis"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"99475","messageId":"cover.1231254832.git.kirr@landau.phys.spbu.ru","threadId":"17023","inReplyTo":null,"subject":"[PATCH (topgit) 0/2] tg-patch: fix pagination","fromName":"Kirill Smelkov","fromEmail":"kirr@landau.phys.spbu.ru","sentAt":"2009-01-06T15:16:37Z","receivedAt":"2009-01-06T15:16:37Z","isPatch":true,"sender":{"key":"kirr@navytux.spb.ru","avatar":"https://gravatar.com/avatar/cf3445fdad1849941e17ab25bf1ee7c5ea1be2deb444be25ff5f36b0e50a985f?d=mp&s=160"},"body":"Kirill Smelkov (2):\n Implement setup_pager just like in git\n tg-patch: fix pagination\n\n tg-patch.sh | 3 +++\n tg.sh | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 56 insertions(+), 0 deletions(-)\n"},{"id":"99476","messageId":"acaae74f79d385014e726b97f8258b2a0caa3dd0.1231254832.git.kirr@landau.phys.spbu.ru","threadId":"17023","inReplyTo":"cover.1231254832.git.kirr@landau.phys.spbu.ru","subject":"[PATCH (topgit) 1/2] Implement setup_pager just like in git","fromName":"Kirill Smelkov","fromEmail":"kirr@landau.phys.spbu.ru","sentAt":"2009-01-06T15:16:38Z","receivedAt":"2009-01-06T15:16:38Z","isPatch":true,"sender":{"key":"kirr@navytux.spb.ru","avatar":"https://gravatar.com/avatar/cf3445fdad1849941e17ab25bf1ee7c5ea1be2deb444be25ff5f36b0e50a985f?d=mp&s=160"},"body":"setup_pager() spawns a pager process and redirect the rest of our output\nto it.\n\nThis will be needed to fix `tg patch` output in the next commit.\n\nSigned-off-by: Kirill Smelkov \n---\n tg.sh | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 53 insertions(+), 0 deletions(-)\n\ndiff --git a/tg.sh b/tg.sh\nindex 8c23d26..51a82af 100644\n--- a/tg.sh\n+++ b/tg.sh\n@@ -243,6 +243,59 @@ do_help()\n \tfi\n }\n \n+## Pager stuff\n+\n+# isatty FD\n+isatty()\n+{\n+\ttty -s 0<&$1 || return 1\n+\treturn 0\n+}\n+\n+# setup_pager\n+# Spawn pager process and redirect the rest of our output to it\n+setup_pager()\n+{\n+\tisatty 1 || return 0\n+\n+\t# TG_PAGER = GIT_PAGER | PAGER\n+\t# http://unix.derkeiler.com/Newsgroups/comp.unix.shell/2004-03/0792.html\n+\tcase ${GIT_PAGER+XXX} in\n+\t'')\n+\t\tcase ${PAGER+XXX} in\n+\t\t'')\n+\t\t\t# both GIT_PAGER & PAGER unset\n+\t\t\tTG_PAGER=''\n+\t\t\t;;\n+\t\t*)\n+\t\t\tTG_PAGER=\"$PAGER\"\n+\t\t\t;;\n+\t\tesac\n+\t\t;;\n+\t*)\n+\t\tTG_PAGER=\"$GIT_PAGER\"\n+\t\t;;\n+\tesac\n+\n+\t[ -z \"$TG_PAGER\" -o \"$TG_PAGER\" = \"cat\" ] && return 0\n+\n+\n+\t# now spawn pager\n+\texport LESS=${LESS:-FRSX}\t# as in pager.c:pager_preexec()\n+\n+\t_pager_fifo=\"$(mktemp -t tg-pager-fifo.XXXXXX)\"\n+\trm \"$_pager_fifo\" && mkfifo -m 600 \"$_pager_fifo\"\n+\n+\t\"$TG_PAGER\" < \"$_pager_fifo\" &\n+\texec > \"$_pager_fifo\"\t\t# dup2(pager_fifo.in, 1)\n+\n+\t# this is needed so e.g. `git diff` will still colorize it's output if\n+\t# requested in ~/.gitconfig with color.diff=auto\n+\texport GIT_PAGER_IN_USE=1\n+\n+\t# atexit(close(1); wait pager)\n+\ttrap \"exec >&-; rm $_pager_fifo; wait\" EXIT\n+}\n \n ## Startup\n \n-- \n1.6.1.48.ge9b8\n"},{"id":"99474","messageId":"7c756270cdd997037616e1601121347b73b2bc77.1231254832.git.kirr@landau.phys.spbu.ru","threadId":"17023","inReplyTo":"cover.1231254832.git.kirr@landau.phys.spbu.ru","subject":"[PATCH (topgit) 2/2] tg-patch: fix pagination","fromName":"Kirill Smelkov","fromEmail":"kirr@landau.phys.spbu.ru","sentAt":"2009-01-06T15:16:39Z","receivedAt":"2009-01-06T15:16:39Z","isPatch":true,"sender":{"key":"kirr@navytux.spb.ru","avatar":"https://gravatar.com/avatar/cf3445fdad1849941e17ab25bf1ee7c5ea1be2deb444be25ff5f36b0e50a985f?d=mp&s=160"},"body":"Previously, when I was invoking `tg patch` the following used to happen:\n\n1. .topmsg content was sent directly to _terminal_\n2. for each file in the patch, its diff was generated with `git diff`\n and sent to *PAGER*\n3. trailing 'tg: ...' was sent to terminal again\n\nSo the problem is that while `tg patch >file` works as expected, plain\n`tg patch` does not -- in pager there is only a part of the whole patch\n(first file diff) and header and trailer are ommitted.\n\nI've finally decided to fix this inconvenience, and the way it works is\nlike in git -- we just hook `setup_pager` function in commands which\nneed to be paginated.\n\nSigned-off-by: Kirill Smelkov \n---\n tg-patch.sh | 3 +++\n 1 files changed, 3 insertions(+), 0 deletions(-)\n\ndiff --git a/tg-patch.sh b/tg-patch.sh\nindex a704375..dc699d2 100644\n--- a/tg-patch.sh\n+++ b/tg-patch.sh\n@@ -24,6 +24,9 @@ done\n base_rev=\"$(git rev-parse --short --verify \"refs/top-bases/$name\" 2>/dev/null)\" ||\n \tdie \"not a TopGit-controlled branch\"\n \n+\n+setup_pager\n+\n git cat-file blob \"$name:.topmsg\"\n echo\n [ -n \"$(git grep '^[-]--' \"$name\" -- \".topmsg\")\" ] || echo '---'\n-- \n1.6.1.48.ge9b8\n"},{"id":"99566","messageId":"20090106203203.GA11274@lapse.rw.madduck.net","threadId":"17023","inReplyTo":"acaae74f79d385014e726b97f8258b2a0caa3dd0.1231254832.git.kirr@landau.phys.spbu.ru","subject":"Re: [PATCH (topgit) 1/2] Implement setup_pager just like in git","fromName":"martin f krafft","fromEmail":"madduck@madduck.net","sentAt":"2009-01-06T20:32:03Z","receivedAt":"2009-01-06T20:32:03Z","isPatch":true,"sender":{"key":"madduck@madduck.net","avatar":null},"body":"Thanks, Kirill, for the patches. A couple of comments inline. I hope\nPetr has a chance to look too.\n\nalso sprach Kirill Smelkov [2009.01.06.1616 +0100]:\n> +# isatty FD\n> +isatty()\n> +{\n> +\ttty -s 0<&$1 || return 1\n> +\treturn 0\n> +}\n\nYou don't need any of the return statements. Functions' return\nvalues are the return values of the last commands they execute.\nSince we are not using set -e, just \"tty -s 0<&$1\" in the body will\nhave the same effect.\n\n> +\t# TG_PAGER = GIT_PAGER | PAGER\n> +\t# http://unix.derkeiler.com/Newsgroups/comp.unix.shell/2004-03/0792.html\n> +\tcase ${GIT_PAGER+XXX} in\n> +\t'')\n> +\t\tcase ${PAGER+XXX} in\n> +\t\t'')\n> +\t\t\t# both GIT_PAGER & PAGER unset\n\nI find this very confusing. Why not simply\n\n TG_PAGER=\"${GIT_PAGER:-}\"\n TG_PAGER=\"${TG_PAGER:-$PAGER}\"\n\n?\n\n> + [ -z \"$TG_PAGER\" -o \"$TG_PAGER\" = \"cat\" ] && return 0\n\nWhat if I set my pager to /bin/cat? But I suppose then there's just\na wasted FIFO and process, nothing big.\n\n> +\t_pager_fifo=\"$(mktemp -t tg-pager-fifo.XXXXXX)\"\n> +\trm \"$_pager_fifo\" && mkfifo -m 600 \"$_pager_fifo\"\n\nThere's a race condition here. I can't see a real problem with it,\nthough, nor do I know of a better way.\n\n> +\t\"$TG_PAGER\" < \"$_pager_fifo\" &\n> +\texec > \"$_pager_fifo\"\t\t# dup2(pager_fifo.in, 1)\n> +\n> +\t# this is needed so e.g. `git diff` will still colorize it's output if\n> +\t# requested in ~/.gitconfig with color.diff=auto\n> +\texport GIT_PAGER_IN_USE=1\n> +\n> +\t# atexit(close(1); wait pager)\n> +\ttrap \"exec >&-; rm $_pager_fifo; wait\" EXIT\n\nConsistency: $_pager_fifo is not passed as a quoted string to rm\nhere. In the unlikely event that $TMPDIR contains a space, this\nwould fail.\n\nI definitely want Petr's opinion on this before I integrate it.\n\n-- \nmartin | http://madduck.net/ | http://two.sentenc.es/\n \nif you see an onion ring -- answer it!\n \nspamtraps: madduck.bogus@madduck.net\n"},{"id":"99567","messageId":"20090107103534.GA30089@chistera.yi.org","threadId":"17023","inReplyTo":"20090106203203.GA11274@lapse.rw.madduck.net","subject":"Re: [PATCH (topgit) 1/2] Implement setup_pager just like in git","fromName":"Adeodato Simó","fromEmail":"dato@net.com.org.es","sentAt":"2009-01-07T10:35:34Z","receivedAt":"2009-01-07T10:35:34Z","isPatch":true,"sender":{"key":"dato@net.com.org.es","avatar":"https://gravatar.com/avatar/952ec7d5d5663eb8baf631b5c37f9c58480a881920dd5f8a2d3a71f969b72b53?d=mp&s=160"},"body":"* martin f krafft [Tue, 06 Jan 2009 21:32:03 +0100]:\n\n> > +\t_pager_fifo=\"$(mktemp -t tg-pager-fifo.XXXXXX)\"\n> > +\trm \"$_pager_fifo\" && mkfifo -m 600 \"$_pager_fifo\"\n\n> There's a race condition here. I can't see a real problem with it,\n> though, nor do I know of a better way.\n\nmktemp -d, and create the fifo inside the directory created by mktemp.\n\n-- \nAdeodato Simó dato at net.com.org.es\nDebian Developer adeodato at debian.org\n \nA hacker does for love what other would not do for money.\n"},{"id":"99571","messageId":"20090107112754.GA15158@roro3","threadId":"17023","inReplyTo":"20090106203203.GA11274@lapse.rw.madduck.net","subject":"Re: [PATCH (topgit) 1/2] Implement setup_pager just like in git","fromName":"Kirill Smelkov","fromEmail":"kirr@landau.phys.spbu.ru","sentAt":"2009-01-07T11:27:54Z","receivedAt":"2009-01-07T11:27:54Z","isPatch":true,"sender":{"key":"kirr@navytux.spb.ru","avatar":"https://gravatar.com/avatar/cf3445fdad1849941e17ab25bf1ee7c5ea1be2deb444be25ff5f36b0e50a985f?d=mp&s=160"},"body":"Martin, thanks for your review.\n\nI'll too reply inline:\n\nOn Tue, Jan 06, 2009 at 09:32:03PM +0100, martin f krafft wrote:\n> Thanks, Kirill, for the patches. A couple of comments inline. I hope\n> Petr has a chance to look too.\n> \n> also sprach Kirill Smelkov [2009.01.06.1616 +0100]:\n> > +# isatty FD\n> > +isatty()\n> > +{\n> > +\ttty -s 0<&$1 || return 1\n> > +\treturn 0\n> > +}\n> \n> You don't need any of the return statements. Functions' return\n> values are the return values of the last commands they execute.\n\nAgree, I'll rework isatty to be just\n\nisatty()\n{\n\ttty -s 0<&$1\n}\n\n> Since we are not using set -e, just \"tty -s 0<&$1\" in the body will\n> have the same effect.\n\nWe _are_ using `set -e` in tg.sh:\n\nhttp://repo.or.cz/w/topgit.git?a=blob;f=tg.sh;h=8c23d26b9a62ddcc1869bb70299862c32edd4403;hb=HEAD#l254\n\nAnd also I think `tty -s 0<&$1` is a bit obscure, so maybe it is still\nbetter to put it into a function with well-known name such as isatty?\n\n> \n> > +\t# TG_PAGER = GIT_PAGER | PAGER\n> > +\t# http://unix.derkeiler.com/Newsgroups/comp.unix.shell/2004-03/0792.html\n> > +\tcase ${GIT_PAGER+XXX} in\n> > +\t'')\n> > +\t\tcase ${PAGER+XXX} in\n> > +\t\t'')\n> > +\t\t\t# both GIT_PAGER & PAGER unset\n> \n> I find this very confusing. Why not simply\n> \n> TG_PAGER=\"${GIT_PAGER:-}\"\n> TG_PAGER=\"${TG_PAGER:-$PAGER}\"\n> \n> ?\n\nI find it confusing too, but this is needed because they usually do\nsomething like this\n\n $ GIT_PAGER='' \n\nto force it to be pagerless.\n\nFor example in git-rebase.sh:\n\nhttp://repo.or.cz/w/git.git?a=blob;f=git-rebase.sh;h=ebd4df3a0e821ddcfd1eabfcaac17f854e172a85;hb=HEAD#l415\n\nAnd other places.\n\n\nSo I think it would be better to preserve the same semantics for `tg\npatch` callers, though it's a pity that it's hard (maybe I'm wrong ?) to\nsee whether an env var is unset.\n\nI'll add additional note about why this is needed.\n\n\n> \n> > + [ -z \"$TG_PAGER\" -o \"$TG_PAGER\" = \"cat\" ] && return 0\n> \n> What if I set my pager to /bin/cat? But I suppose then there's just\n> a wasted FIFO and process, nothing big.\n\nYes. I'd drop \"cat\" from here completely, but since I was mimicing\npager.c I left it:\n\nhttp://repo.or.cz/w/git.git?a=blob;f=pager.c;h=f19ddbc87df04f117cd5e39189c8322fd5f29d68;hb=HEAD#l55\n\nI'm ok with dropping cat. Should we?\n\n> > +\t_pager_fifo=\"$(mktemp -t tg-pager-fifo.XXXXXX)\"\n> > +\trm \"$_pager_fifo\" && mkfifo -m 600 \"$_pager_fifo\"\n> \n> There's a race condition here. I can't see a real problem with it,\n> though, nor do I know of a better way.\n\nYes I know and agree here is a race.\n\nBut netheir did I knew how to overcome this (I though we'll need mkfifo\nto support creating fifos with temp names, but mkfifo lacks support for\nthis)\n\nThe real problem here is that in bash, it's hard to get such constructs\ngoing without fifos at all -- just using pipes like we would do in C,\nbecause:\n\n exec >file\n\nredirects the rest of current process output to file, but\n\n exec |less\n\ndoes _not_ redirect the rest of the current process output through pipe\nto less.\n\nLikewise\n\n exec > >(less)\n\nworks (process redirection), but is a bashishm, so = not an option for\nus.\n\nFortunately Adeodato suggested to use `mktemp -d`, so this is reworked\nto:\n\n _pager_fifo_dir=\"$(mktemp -t -d tg-pager-fifo.XXXXXX)\"\n _pager_fifo=\"$_pager_fifo_dir/0\"\n mkfifo -m 600 \"$_pager_fifo\"\n\n+ appopriate cleanup.\n\n> > +\t\"$TG_PAGER\" < \"$_pager_fifo\" &\n> > +\texec > \"$_pager_fifo\"\t\t# dup2(pager_fifo.in, 1)\n> > +\n> > +\t# this is needed so e.g. `git diff` will still colorize it's output if\n> > +\t# requested in ~/.gitconfig with color.diff=auto\n> > +\texport GIT_PAGER_IN_USE=1\n> > +\n> > +\t# atexit(close(1); wait pager)\n> > +\ttrap \"exec >&-; rm $_pager_fifo; wait\" EXIT\n> \n> Consistency: $_pager_fifo is not passed as a quoted string to rm\n> here. In the unlikely event that $TMPDIR contains a space, this\n> would fail.\n\nThanks, corrected.\n\n> I definitely want Petr's opinion on this before I integrate it.\n\nOk, let's wait what Petr says. Here is improved patch:\n\nChanges since v1:\n\n o isatty() simplified\n o added a note about different meaning of GIT_PAGER='' and unset\n GIT_PAGER\n o fixed race wrt to mkfifo creation\n o fixed potential whitespace problem in \"$_pager_fifo\" cleanup\n\n\n(interdiff)\n\ndiff --git a/tg.sh b/tg.sh\nindex 51a82af..bf9cf5c 100644\n--- a/tg.sh\n+++ b/tg.sh\n@@ -248,8 +248,7 @@ do_help()\n # isatty FD\n isatty()\n {\n-\ttty -s 0<&$1 || return 1\n-\treturn 0\n+\ttty -s 0<&$1\n }\n \n # setup_pager\n@@ -259,6 +258,7 @@ setup_pager()\n \tisatty 1 || return 0\n \n \t# TG_PAGER = GIT_PAGER | PAGER\n+\t# (but differentiate between GIT_PAGER='' and unset variables)\n \t# http://unix.derkeiler.com/Newsgroups/comp.unix.shell/2004-03/0792.html\n \tcase ${GIT_PAGER+XXX} in\n \t'')\n@@ -283,8 +283,9 @@ setup_pager()\n \t# now spawn pager\n \texport LESS=${LESS:-FRSX}\t# as in pager.c:pager_preexec()\n \n-\t_pager_fifo=\"$(mktemp -t tg-pager-fifo.XXXXXX)\"\n-\trm \"$_pager_fifo\" && mkfifo -m 600 \"$_pager_fifo\"\n+\t_pager_fifo_dir=\"$(mktemp -t -d tg-pager-fifo.XXXXXX)\"\n+\t_pager_fifo=\"$_pager_fifo_dir/0\"\n+\tmkfifo -m 600 \"$_pager_fifo\"\n \n \t\"$TG_PAGER\" < \"$_pager_fifo\" &\n \texec > \"$_pager_fifo\"\t\t# dup2(pager_fifo.in, 1)\n@@ -294,7 +295,7 @@ setup_pager()\n \texport GIT_PAGER_IN_USE=1\n \n \t# atexit(close(1); wait pager)\n-\ttrap \"exec >&-; rm $_pager_fifo; wait\" EXIT\n+\ttrap \"exec >&-; rm \"$_pager_fifo\"; rmdir \"$_pager_fifo_dir\"; wait\" EXIT\n }\n \n ## Startup\n\n\n>From 2193b7c703c2d31c8739eec617b8c0e8c1d09b79 Mon Sep 17 00:00:00 2001\nFrom: Kirill Smelkov \nDate: Tue, 6 Jan 2009 17:56:37 +0300\nSubject: [PATCH (topgit) v2] Implement setup_pager just like in git\n\nsetup_pager() spawns a pager process and redirect the rest of our output\nto it.\n\nThis will be needed to fix `tg patch` output in the next commit.\n\nSigned-off-by: Kirill Smelkov \n---\n tg.sh | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 54 insertions(+), 0 deletions(-)\n\ndiff --git a/tg.sh b/tg.sh\nindex 8c23d26..bf9cf5c 100644\n--- a/tg.sh\n+++ b/tg.sh\n@@ -243,6 +243,60 @@ do_help()\n \tfi\n }\n \n+## Pager stuff\n+\n+# isatty FD\n+isatty()\n+{\n+\ttty -s 0<&$1\n+}\n+\n+# setup_pager\n+# Spawn pager process and redirect the rest of our output to it\n+setup_pager()\n+{\n+\tisatty 1 || return 0\n+\n+\t# TG_PAGER = GIT_PAGER | PAGER\n+\t# (but differentiate between GIT_PAGER='' and unset variables)\n+\t# http://unix.derkeiler.com/Newsgroups/comp.unix.shell/2004-03/0792.html\n+\tcase ${GIT_PAGER+XXX} in\n+\t'')\n+\t\tcase ${PAGER+XXX} in\n+\t\t'')\n+\t\t\t# both GIT_PAGER & PAGER unset\n+\t\t\tTG_PAGER=''\n+\t\t\t;;\n+\t\t*)\n+\t\t\tTG_PAGER=\"$PAGER\"\n+\t\t\t;;\n+\t\tesac\n+\t\t;;\n+\t*)\n+\t\tTG_PAGER=\"$GIT_PAGER\"\n+\t\t;;\n+\tesac\n+\n+\t[ -z \"$TG_PAGER\" -o \"$TG_PAGER\" = \"cat\" ] && return 0\n+\n+\n+\t# now spawn pager\n+\texport LESS=${LESS:-FRSX}\t# as in pager.c:pager_preexec()\n+\n+\t_pager_fifo_dir=\"$(mktemp -t -d tg-pager-fifo.XXXXXX)\"\n+\t_pager_fifo=\"$_pager_fifo_dir/0\"\n+\tmkfifo -m 600 \"$_pager_fifo\"\n+\n+\t\"$TG_PAGER\" < \"$_pager_fifo\" &\n+\texec > \"$_pager_fifo\"\t\t# dup2(pager_fifo.in, 1)\n+\n+\t# this is needed so e.g. `git diff` will still colorize it's output if\n+\t# requested in ~/.gitconfig with color.diff=auto\n+\texport GIT_PAGER_IN_USE=1\n+\n+\t# atexit(close(1); wait pager)\n+\ttrap \"exec >&-; rm \"$_pager_fifo\"; rmdir \"$_pager_fifo_dir\"; wait\" EXIT\n+}\n \n ## Startup\n \n-- \n1.6.1.48.ge9b8\n\n\n\nThanks,\nKirill\n"},{"id":"99572","messageId":"200901071324.57222.trast@student.ethz.ch","threadId":"17023","inReplyTo":"20090107112754.GA15158@roro3","subject":"Re: [PATCH (topgit) 1/2] Implement setup_pager just like in git","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2009-01-07T12:24:44Z","receivedAt":"2009-01-07T12:24:44Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Kirill Smelkov wrote:\n> On Tue, Jan 06, 2009 at 09:32:03PM +0100, martin f krafft wrote:\n> > I find this very confusing. Why not simply\n> > \n> > TG_PAGER=\"${GIT_PAGER:-}\"\n> > TG_PAGER=\"${TG_PAGER:-$PAGER}\"\n> > \n> > ?\n> \n> I find it confusing too, but this is needed because they usually do\n> something like this\n> \n> $ GIT_PAGER='' \n> \n> to force it to be pagerless.\n[...]\n> So I think it would be better to preserve the same semantics for `tg\n> patch` callers, though it's a pity that it's hard (maybe I'm wrong ?) to\n> see whether an env var is unset.\n\nAdmittedly I haven't really studied your patch, but the ${} constructs\ncan in fact tell empty from unset:\n\n $ EMPTY=\n $ unset UNDEFINED\n $ echo ${UNDEFINED-foo}\n foo\n $ echo ${UNDEFINED:-foo}\n foo\n $ echo ${EMPTY-foo}\n\n $ echo ${EMPTY:-foo}\n foo\n\n'man bash' indeed says\n\n When not performing substring expansion, bash tests for a parameter\n that is unset or null; omitting the colon results in a test only for\n a parameter that is unset.\n\nSo I suppose you could use\n\n ${GIT_PAGER-${PAGER-less}}\n\nor similar.\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n\n"},{"id":"99580","messageId":"36ca99e90901070624p2c102f3ey392ef813db9f9187@mail.gmail.com","threadId":"17023","inReplyTo":"20090107112754.GA15158@roro3","subject":"Re: [PATCH (topgit) 1/2] Implement setup_pager just like in git","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2009-01-07T14:24:02Z","receivedAt":"2009-01-07T14:24:02Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"On Wed, Jan 7, 2009 at 12:27, Kirill Smelkov wrote:\n> Martin, thanks for your review.\n> + # atexit(close(1); wait pager)\n> + trap \"exec >&-; rm \"$_pager_fifo\"; rmdir \"$_pager_fifo_dir\"; wait\" EXIT\nI think you need to escape the double quotes.\n\nBert\n"},{"id":"99582","messageId":"20090107144432.GC831@artemis.corp","threadId":"17023","inReplyTo":"20090107112754.GA15158@roro3","subject":"Re: [PATCH (topgit) 1/2] Implement setup_pager just like in git","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2009-01-07T14:44:32Z","receivedAt":"2009-01-07T14:44:32Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Wed, Jan 07, 2009 at 11:27:54AM +0000, Kirill Smelkov wrote:\n> Martin, thanks for your review.\n> \n> I'll too reply inline:\n> \n> On Tue, Jan 06, 2009 at 09:32:03PM +0100, martin f krafft wrote:\n> > Thanks, Kirill, for the patches. A couple of comments inline. I hope\n> > Petr has a chance to look too.\n> > \n> > also sprach Kirill Smelkov [2009.01.06.1616 +0100]:\n> > > +# isatty FD\n> > > +isatty()\n> > > +{\n> > > +\ttty -s 0<&$1 || return 1\n> > > +\treturn 0\n> > > +}\n> > \n> > You don't need any of the return statements. Functions' return\n> > values are the return values of the last commands they execute.\n> \n> Agree, I'll rework isatty to be just\n> \n> isatty()\n> {\n> \ttty -s 0<&$1\n> }\n\nwhy not test -t 0 ? I'm not sure it's POSIX though.\n\n\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"99586","messageId":"20090107151000.GR12275@machine.or.cz","threadId":"17023","inReplyTo":"20090107144432.GC831@artemis.corp","subject":"Re: [PATCH (topgit) 1/2] Implement setup_pager just like in git","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2009-01-07T15:10:00Z","receivedAt":"2009-01-07T15:10:00Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"On Wed, Jan 07, 2009 at 02:27:54PM +0300, Kirill Smelkov wrote:\n> >From 2193b7c703c2d31c8739eec617b8c0e8c1d09b79 Mon Sep 17 00:00:00 2001\n> From: Kirill Smelkov \n> Date: Tue, 6 Jan 2009 17:56:37 +0300\n> Subject: [PATCH (topgit) v2] Implement setup_pager just like in git\n> \n> setup_pager() spawns a pager process and redirect the rest of our output\n> to it.\n> \n> This will be needed to fix `tg patch` output in the next commit.\n> \n> Signed-off-by: Kirill Smelkov \n\nBut you never use it...?\n\n> ---\n> tg.sh | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++\n> 1 files changed, 54 insertions(+), 0 deletions(-)\n> \n> diff --git a/tg.sh b/tg.sh\n> index 8c23d26..bf9cf5c 100644\n> --- a/tg.sh\n> +++ b/tg.sh\n> @@ -243,6 +243,60 @@ do_help()\n> \tfi\n> }\n> \n> +## Pager stuff\n> +\n> +# isatty FD\n> +isatty()\n> +{\n> +\ttty -s 0<&$1\n> +}\n> +\n> +# setup_pager\n> +# Spawn pager process and redirect the rest of our output to it\n> +setup_pager()\n> +{\n> +\tisatty 1 || return 0\n> +\n> +\t# TG_PAGER = GIT_PAGER | PAGER\n> +\t# (but differentiate between GIT_PAGER='' and unset variables)\n> +\t# http://unix.derkeiler.com/Newsgroups/comp.unix.shell/2004-03/0792.html\n> +\tcase ${GIT_PAGER+XXX} in\n> +\t'')\n> +\t\tcase ${PAGER+XXX} in\n> +\t\t'')\n\nI'm pretty sure there's been a nice trick for this, but I can't remember\nit at all now.\n\n> +\t\t\t# both GIT_PAGER & PAGER unset\n> +\t\t\tTG_PAGER=''\n> +\t\t\t;;\n> +\t\t*)\n> +\t\t\tTG_PAGER=\"$PAGER\"\n> +\t\t\t;;\n> +\t\tesac\n> +\t\t;;\n> +\t*)\n> +\t\tTG_PAGER=\"$GIT_PAGER\"\n> +\t\t;;\n> +\tesac\n> +\n> +\t[ -z \"$TG_PAGER\" -o \"$TG_PAGER\" = \"cat\" ] && return 0\n> +\n> +\n> +\t# now spawn pager\n> +\texport LESS=${LESS:-FRSX}\t# as in pager.c:pager_preexec()\n> +\n> +\t_pager_fifo_dir=\"$(mktemp -t -d tg-pager-fifo.XXXXXX)\"\n> +\t_pager_fifo=\"$_pager_fifo_dir/0\"\n> +\tmkfifo -m 600 \"$_pager_fifo\"\n> +\n> +\t\"$TG_PAGER\" < \"$_pager_fifo\" &\n> +\texec > \"$_pager_fifo\"\t\t# dup2(pager_fifo.in, 1)\n> +\n> +\t# this is needed so e.g. `git diff` will still colorize it's output if\n> +\t# requested in ~/.gitconfig with color.diff=auto\n> +\texport GIT_PAGER_IN_USE=1\n> +\n> +\t# atexit(close(1); wait pager)\n> +\ttrap \"exec >&-; rm \"$_pager_fifo\"; rmdir \"$_pager_fifo_dir\"; wait\" EXIT\n> +}\n\nFrankly, I would have been just much happier if something like git\npager--helper would be provided for external tools to use. Seeing how it\ngets reimplemented like this just pains me greatly.\n\nOn Wed, Jan 07, 2009 at 03:44:32PM +0100, Pierre Habouzit wrote:\n> On Wed, Jan 07, 2009 at 11:27:54AM +0000, Kirill Smelkov wrote:\n> > isatty()\n> > {\n> > \ttty -s 0<&$1\n> > }\n> \n> why not test -t 0 ? I'm not sure it's POSIX though.\n\nIt's SUS for many issues already it seems.\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nThe average, healthy, well-adjusted adult gets up at seven-thirty\nin the morning feeling just terrible. -- Jean Kerr\n"},{"id":"99620","messageId":"20090107220027.GA4946@roro3","threadId":"17023","inReplyTo":"20090107151000.GR12275@machine.or.cz","subject":"Re: [PATCH (topgit) 1/2] Implement setup_pager just like in git","fromName":"Kirill Smelkov","fromEmail":"kirr@landau.phys.spbu.ru","sentAt":"2009-01-07T22:00:27Z","receivedAt":"2009-01-07T22:00:27Z","isPatch":true,"sender":{"key":"kirr@navytux.spb.ru","avatar":"https://gravatar.com/avatar/cf3445fdad1849941e17ab25bf1ee7c5ea1be2deb444be25ff5f36b0e50a985f?d=mp&s=160"},"body":"On Wed, Jan 07, 2009 at 01:24:44PM +0100, Thomas Rast wrote:\n> Kirill Smelkov wrote:\n> > On Tue, Jan 06, 2009 at 09:32:03PM +0100, martin f krafft wrote:\n> > > I find this very confusing. Why not simply\n> > > \n> > > TG_PAGER=\"${GIT_PAGER:-}\"\n> > > TG_PAGER=\"${TG_PAGER:-$PAGER}\"\n> > > \n> > > ?\n> > \n> > I find it confusing too, but this is needed because they usually do\n> > something like this\n> > \n> > $ GIT_PAGER='' \n> > \n> > to force it to be pagerless.\n> [...]\n> > So I think it would be better to preserve the same semantics for `tg\n> > patch` callers, though it's a pity that it's hard (maybe I'm wrong ?) to\n> > see whether an env var is unset.\n> \n> Admittedly I haven't really studied your patch, but the ${} constructs\n> can in fact tell empty from unset:\n> \n> $ EMPTY=\n> $ unset UNDEFINED\n> $ echo ${UNDEFINED-foo}\n> foo\n> $ echo ${UNDEFINED:-foo}\n> foo\n> $ echo ${EMPTY-foo}\n> \n> $ echo ${EMPTY:-foo}\n> foo\n> \n> 'man bash' indeed says\n> \n> When not performing substring expansion, bash tests for a parameter\n> that is unset or null; omitting the colon results in a test only for\n> a parameter that is unset.\n> \n> So I suppose you could use\n> \n> ${GIT_PAGER-${PAGER-less}}\n> \n> or similar.\n\nGood eyes, thanks!\n\nI'll rework it.\n\n\nOn Wed, Jan 07, 2009 at 03:24:02PM +0100, Bert Wesarg wrote:\n> On Wed, Jan 7, 2009 at 12:27, Kirill Smelkov wrote:\n> > Martin, thanks for your review.\n> > + # atexit(close(1); wait pager)\n> > + trap \"exec >&-; rm \"$_pager_fifo\"; rmdir \"$_pager_fifo_dir\"; wait\" EXIT\n> I think you need to escape the double quotes.\n\nGood eyes -- corrected and thanks!\n\n\nOn Wed, Jan 07, 2009 at 04:10:00PM +0100, Petr Baudis wrote:\n> On Wed, Jan 07, 2009 at 02:27:54PM +0300, Kirill Smelkov wrote:\n> > >From 2193b7c703c2d31c8739eec617b8c0e8c1d09b79 Mon Sep 17 00:00:00 2001\n> > From: Kirill Smelkov \n> > Date: Tue, 6 Jan 2009 17:56:37 +0300\n> > Subject: [PATCH (topgit) v2] Implement setup_pager just like in git\n> > \n> > setup_pager() spawns a pager process and redirect the rest of our output\n> > to it.\n> > \n> > This will be needed to fix `tg patch` output in the next commit.\n> > \n> > Signed-off-by: Kirill Smelkov \n> \n> But you never use it...?\n\nWhat do you mean?\n\nIt is used in the next patch as posted in original series:\n\nhttp://marc.info/?l=git&m=123125495000600&w=2\n\nFor completeness, I'll include both patches in this email.\n\n> > ---\n> > tg.sh | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++\n> > 1 files changed, 54 insertions(+), 0 deletions(-)\n> > \n> > diff --git a/tg.sh b/tg.sh\n> > index 8c23d26..bf9cf5c 100644\n> > --- a/tg.sh\n> > +++ b/tg.sh\n> > @@ -243,6 +243,60 @@ do_help()\n> > \tfi\n> > }\n> > \n> > +## Pager stuff\n> > +\n> > +# isatty FD\n> > +isatty()\n> > +{\n> > +\ttty -s 0<&$1\n> > +}\n> > +\n> > +# setup_pager\n> > +# Spawn pager process and redirect the rest of our output to it\n> > +setup_pager()\n> > +{\n> > +\tisatty 1 || return 0\n> > +\n> > +\t# TG_PAGER = GIT_PAGER | PAGER\n> > +\t# (but differentiate between GIT_PAGER='' and unset variables)\n> > +\t# http://unix.derkeiler.com/Newsgroups/comp.unix.shell/2004-03/0792.html\n> > +\tcase ${GIT_PAGER+XXX} in\n> > +\t'')\n> > +\t\tcase ${PAGER+XXX} in\n> > +\t\t'')\n> \n> I'm pretty sure there's been a nice trick for this, but I can't remember\n> it at all now.\n\nAlready corrected to ${GIT_PAGER-${PAGER-less}}, thanks to Thomas.\n\n> > +\t\t\t# both GIT_PAGER & PAGER unset\n> > +\t\t\tTG_PAGER=''\n> > +\t\t\t;;\n> > +\t\t*)\n> > +\t\t\tTG_PAGER=\"$PAGER\"\n> > +\t\t\t;;\n> > +\t\tesac\n> > +\t\t;;\n> > +\t*)\n> > +\t\tTG_PAGER=\"$GIT_PAGER\"\n> > +\t\t;;\n> > +\tesac\n> > +\n> > +\t[ -z \"$TG_PAGER\" -o \"$TG_PAGER\" = \"cat\" ] && return 0\n> > +\n> > +\n> > +\t# now spawn pager\n> > +\texport LESS=${LESS:-FRSX}\t# as in pager.c:pager_preexec()\n> > +\n> > +\t_pager_fifo_dir=\"$(mktemp -t -d tg-pager-fifo.XXXXXX)\"\n> > +\t_pager_fifo=\"$_pager_fifo_dir/0\"\n> > +\tmkfifo -m 600 \"$_pager_fifo\"\n> > +\n> > +\t\"$TG_PAGER\" < \"$_pager_fifo\" &\n> > +\texec > \"$_pager_fifo\"\t\t# dup2(pager_fifo.in, 1)\n> > +\n> > +\t# this is needed so e.g. `git diff` will still colorize it's output if\n> > +\t# requested in ~/.gitconfig with color.diff=auto\n> > +\texport GIT_PAGER_IN_USE=1\n> > +\n> > +\t# atexit(close(1); wait pager)\n> > +\ttrap \"exec >&-; rm \"$_pager_fifo\"; rmdir \"$_pager_fifo_dir\"; wait\" EXIT\n> > +}\n> \n> Frankly, I would have been just much happier if something like git\n> pager--helper would be provided for external tools to use. Seeing how it\n> gets reimplemented like this just pains me greatly.\n\nAfter we settle on implementation, would it make sense to include this\nsetup_pager into git-sh-setup?\n\nI propose we include this stuff into tg.sh first, so that topgit would\nwork correctly with previous versions of git.\n\n> On Wed, Jan 07, 2009 at 03:44:32PM +0100, Pierre Habouzit wrote:\n> > On Wed, Jan 07, 2009 at 11:27:54AM +0000, Kirill Smelkov wrote:\n> > > isatty()\n> > > {\n> > > \ttty -s 0<&$1\n> > > }\n> > \n> > why not test -t 0 ? I'm not sure it's POSIX though.\n> \n> It's SUS for many issues already it seems.\n\nPierre and Petr - thanks for the info. Yes, `test -t $fd` looks better.\n\n\nHere is improved patch:\n\nChanges since v1:\n\n o Simplify TG_PAGER setup (thanks to Thomas Rast)\n o Properly escape \" (thanks to Bert Wesarg)\n o Simpler isatty (thanks to Pierre Habouzit & Petr Baudis)\n\n\n(interdiff)\n\ndiff --git a/tg.sh b/tg.sh\nindex bf9cf5c..b64fc3a 100644\n--- a/tg.sh\n+++ b/tg.sh\n@@ -248,7 +248,7 @@ do_help()\n # isatty FD\n isatty()\n {\n-\ttty -s 0<&$1\n+\ttest -t $1\n }\n \n # setup_pager\n@@ -257,25 +257,9 @@ setup_pager()\n {\n \tisatty 1 || return 0\n \n-\t# TG_PAGER = GIT_PAGER | PAGER\n-\t# (but differentiate between GIT_PAGER='' and unset variables)\n-\t# http://unix.derkeiler.com/Newsgroups/comp.unix.shell/2004-03/0792.html\n-\tcase ${GIT_PAGER+XXX} in\n-\t'')\n-\t\tcase ${PAGER+XXX} in\n-\t\t'')\n-\t\t\t# both GIT_PAGER & PAGER unset\n-\t\t\tTG_PAGER=''\n-\t\t\t;;\n-\t\t*)\n-\t\t\tTG_PAGER=\"$PAGER\"\n-\t\t\t;;\n-\t\tesac\n-\t\t;;\n-\t*)\n-\t\tTG_PAGER=\"$GIT_PAGER\"\n-\t\t;;\n-\tesac\n+\t# TG_PAGER = GIT_PAGER | PAGER | less\n+\t# NOTE: GIT_PAGER='' is significant\n+\tTG_PAGER=${GIT_PAGER-${PAGER-less}}\n \n \t[ -z \"$TG_PAGER\" -o \"$TG_PAGER\" = \"cat\" ] && return 0\n \n@@ -295,7 +279,7 @@ setup_pager()\n \texport GIT_PAGER_IN_USE=1\n \n \t# atexit(close(1); wait pager)\n-\ttrap \"exec >&-; rm \"$_pager_fifo\"; rmdir \"$_pager_fifo_dir\"; wait\" EXIT\n+\ttrap \"exec >&-; rm \\\"$_pager_fifo\\\"; rmdir \\\"$_pager_fifo_dir\\\"; wait\" EXIT\n }\n \n ## Startup\n\n\nFrom: Kirill Smelkov \nTo: Petr Baudis \nCc: Git Mailing List \nBcc: Kirill Smelkov \nSubject: Implement setup_pager just like in git\n\nsetup_pager() spawns a pager process and redirect the rest of our output\nto it.\n\nThis will be needed to fix `tg patch` output in the next commit.\n\nSigned-off-by: Kirill Smelkov \n\n---\n tg.sh | 38 ++++++++++++++++++++++++++++++++++++++\n 1 files changed, 38 insertions(+), 0 deletions(-)\n\ndiff --git a/tg.sh b/tg.sh\nindex 8c23d26..b64fc3a 100644\n--- a/tg.sh\n+++ b/tg.sh\n@@ -243,6 +243,44 @@ do_help()\n \tfi\n }\n \n+## Pager stuff\n+\n+# isatty FD\n+isatty()\n+{\n+\ttest -t $1\n+}\n+\n+# setup_pager\n+# Spawn pager process and redirect the rest of our output to it\n+setup_pager()\n+{\n+\tisatty 1 || return 0\n+\n+\t# TG_PAGER = GIT_PAGER | PAGER | less\n+\t# NOTE: GIT_PAGER='' is significant\n+\tTG_PAGER=${GIT_PAGER-${PAGER-less}}\n+\n+\t[ -z \"$TG_PAGER\" -o \"$TG_PAGER\" = \"cat\" ] && return 0\n+\n+\n+\t# now spawn pager\n+\texport LESS=${LESS:-FRSX}\t# as in pager.c:pager_preexec()\n+\n+\t_pager_fifo_dir=\"$(mktemp -t -d tg-pager-fifo.XXXXXX)\"\n+\t_pager_fifo=\"$_pager_fifo_dir/0\"\n+\tmkfifo -m 600 \"$_pager_fifo\"\n+\n+\t\"$TG_PAGER\" < \"$_pager_fifo\" &\n+\texec > \"$_pager_fifo\"\t\t# dup2(pager_fifo.in, 1)\n+\n+\t# this is needed so e.g. `git diff` will still colorize it's output if\n+\t# requested in ~/.gitconfig with color.diff=auto\n+\texport GIT_PAGER_IN_USE=1\n+\n+\t# atexit(close(1); wait pager)\n+\ttrap \"exec >&-; rm \\\"$_pager_fifo\\\"; rmdir \\\"$_pager_fifo_dir\\\"; wait\" EXIT\n+}\n \n ## Startup\n \n-- \ntg: (8c77c34..) t/setup-pager (depends on: master)\n\n\nSecond patch which uses setup_pager:\n\n\n>From 1b723ebf740c58bc25ac97eff0a31b07373d8d1e Mon Sep 17 00:00:00 2001\nFrom: Kirill Smelkov \nDate: Tue, 6 Jan 2009 18:03:21 +0300\nSubject: [TopGit PATCH] tg-patch: fix pagination\n\nPreviously, when I was invoking `tg patch` the following used to happen:\n\n1. .topmsg content was sent directly to _terminal_\n2. for each file in the patch, its diff was generated with `git diff`\n and sent to *PAGER*\n3. trailing 'tg: ...' was sent to terminal again\n\nSo the problem is that while `tg patch >file` works as expected, plain\n`tg patch` does not -- in pager there is only a part of the whole patch\n(first file diff) and header and trailer are ommitted.\n\nI've finally decided to fix this inconvenience, and the way it works is\nlike in git -- we just hook `setup_pager` function in commands which\nneed to be paginated.\n\nSigned-off-by: Kirill Smelkov \n---\n tg-patch.sh | 3 +++\n 1 files changed, 3 insertions(+), 0 deletions(-)\n\ndiff --git a/tg-patch.sh b/tg-patch.sh\nindex a704375..dc699d2 100644\n--- a/tg-patch.sh\n+++ b/tg-patch.sh\n@@ -24,6 +24,9 @@ done\n base_rev=\"$(git rev-parse --short --verify \"refs/top-bases/$name\" 2>/dev/null)\" ||\n \tdie \"not a TopGit-controlled branch\"\n \n+\n+setup_pager\n+\n git cat-file blob \"$name:.topmsg\"\n echo\n [ -n \"$(git grep '^[-]--' \"$name\" -- \".topmsg\")\" ] || echo '---'\n-- \n1.6.1.48.ge9b8\n\n\nThanks,\nKirill\n"},{"id":"99668","messageId":"20090108020650.GC7345@lapse.rw.madduck.net","threadId":"17023","inReplyTo":"20090107220027.GA4946@roro3","subject":"Re: [PATCH (topgit) 1/2] Implement setup_pager just like in git","fromName":"martin f krafft","fromEmail":"madduck@madduck.net","sentAt":"2009-01-08T02:06:50Z","receivedAt":"2009-01-08T02:06:50Z","isPatch":true,"sender":{"key":"madduck@madduck.net","avatar":null},"body":"also sprach Kirill Smelkov [2009.01.08.1100 +1300]:\n> > So I suppose you could use\n> > \n> > ${GIT_PAGER-${PAGER-less}}\n> > \n> > or similar.\n> \n> Good eyes, thanks!\n> \n> I'll rework it.\n\nI am not 100% on this, but I think nested {}'s are a bashism.\n\n> On Wed, Jan 07, 2009 at 03:24:02PM +0100, Bert Wesarg wrote:\n> > On Wed, Jan 7, 2009 at 12:27, Kirill Smelkov wrote:\n> > > Martin, thanks for your review.\n> > > + # atexit(close(1); wait pager)\n> > > + trap \"exec >&-; rm \"$_pager_fifo\"; rmdir \"$_pager_fifo_dir\"; wait\" EXIT\n> > I think you need to escape the double quotes.\n> \n> Good eyes -- corrected and thanks!\n\nYou could also just use single quotes inside the double quotes.\n\n-- \nmartin | http://madduck.net/ | http://two.sentenc.es/\n \n\"he gave me his card\n he said, 'call me if they die'\n i shook his hand and said goodbye\n ran out to the street\n when a bowling ball came down the road\n and knocked me off my feet\"\n -- bob dylan\n \nspamtraps: madduck.bogus@madduck.net\n"},{"id":"99687","messageId":"20090108092304.GA4835@roro3","threadId":"17023","inReplyTo":"20090108020650.GC7345@lapse.rw.madduck.net","subject":"Re: [PATCH (topgit) 1/2] Implement setup_pager just like in git","fromName":"Kirill Smelkov","fromEmail":"kirr@landau.phys.spbu.ru","sentAt":"2009-01-08T09:23:04Z","receivedAt":"2009-01-08T09:23:04Z","isPatch":true,"sender":{"key":"kirr@navytux.spb.ru","avatar":"https://gravatar.com/avatar/cf3445fdad1849941e17ab25bf1ee7c5ea1be2deb444be25ff5f36b0e50a985f?d=mp&s=160"},"body":"On Thu, Jan 08, 2009 at 03:06:50PM +1300, martin f krafft wrote:\n> also sprach Kirill Smelkov [2009.01.08.1100 +1300]:\n> > > So I suppose you could use\n> > > \n> > > ${GIT_PAGER-${PAGER-less}}\n> > > \n> > > or similar.\n> > \n> > Good eyes, thanks!\n> > \n> > I'll rework it.\n> \n> I am not 100% on this, but I think nested {}'s are a bashism.\n\nIt seems to be ok:\n\nkirr@roro3:~$ dash \n$ unset GIT_PAGER\n$ unset PAGER\n$ echo ${GIT_PAGER-${PAGER-less}}\nless\n$ PAGER=more\n$ echo ${GIT_PAGER-${PAGER-less}}\nmore\n$ GIT_PAGER=''\n$ echo ${GIT_PAGER-${PAGER-less}}\n\n$ GIT_PAGER=/bin/cat\n$ echo ${GIT_PAGER-${PAGER-less}}\n/bin/cat\n\n\n> > On Wed, Jan 07, 2009 at 03:24:02PM +0100, Bert Wesarg wrote:\n> > > On Wed, Jan 7, 2009 at 12:27, Kirill Smelkov wrote:\n> > > > Martin, thanks for your review.\n> > > > + # atexit(close(1); wait pager)\n> > > > + trap \"exec >&-; rm \"$_pager_fifo\"; rmdir \"$_pager_fifo_dir\"; wait\" EXIT\n> > > I think you need to escape the double quotes.\n> > \n> > Good eyes -- corrected and thanks!\n> \n> You could also just use single quotes inside the double quotes.\n\nThanks for the tip - I'll keep it in mind. Or is it the preferred way?\n\n\nThanks,\nKirill\n"},{"id":"100984","messageId":"20090118150420.GB27522@roro3.zxlink","threadId":"17023","inReplyTo":"cover.1231254832.git.kirr@landau.phys.spbu.ru","subject":"Re: [PATCH (topgit) 0/2] tg-patch: fix pagination","fromName":"Kirill Smelkov","fromEmail":"kirr@landau.phys.spbu.ru","sentAt":"2009-01-18T15:04:20Z","receivedAt":"2009-01-18T15:04:20Z","isPatch":true,"sender":{"key":"kirr@navytux.spb.ru","avatar":"https://gravatar.com/avatar/cf3445fdad1849941e17ab25bf1ee7c5ea1be2deb444be25ff5f36b0e50a985f?d=mp&s=160"},"body":"On Tue, Jan 06, 2009 at 06:16:37PM +0300, Kirill Smelkov wrote:\n> Kirill Smelkov (2):\n> Implement setup_pager just like in git\n> tg-patch: fix pagination\n> \n> tg-patch.sh | 3 +++\n> tg.sh | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++\n> 2 files changed, 56 insertions(+), 0 deletions(-)\n\nPetr, Martin,\n\nWhat's the state of this patch series?\n\nIt fixes `tg patch`, and it seems we addressed all uglyness in\nsetup_pager, so is there any other reason not to merge this?\n\nThanks,\nKirill\n"}]}