{"thread":{"id":"28411","subject":"[RFC/PATCH] Configurable hyperlinking in gitk","startedAt":"2011-09-17T02:29:03Z","lastAt":"2011-10-12T09:07:28Z","messageCount":33,"participants":["Jeff Epler","Chris Packham","Jeff King","Christian Couder","Jakub Narebski","Marc Branchaud","Junio C Hamano","Andreas Schwab"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"175680","messageId":"20110917022903.GA2445@unpythonic.net","threadId":"28411","inReplyTo":null,"subject":"[RFC/PATCH] Configurable hyperlinking in gitk","fromName":"Jeff Epler","fromEmail":"jepler@unpythonic.net","sentAt":"2011-09-17T02:29:03Z","receivedAt":"2011-09-17T02:29:03Z","isPatch":true,"sender":{"key":"jepler@unpythonic.net","avatar":"https://avatars.githubusercontent.com/u/1517291?v=4"},"body":"Many projects use project-specific notations in comments to refer to\nbug trackers and the like. One example is the \"Closes: #nnnnn\"\nnotation used in Debian.\n\nMake gitk configurable so that arbitrary strings can be turned into\nclickable links that are opened in a web browser.\n---\nSome time ago I hardcoded this into gitk for $DAY_JOB and find it very\nuseful. I made it configurable in the hopes that it might be adopted\nupstream. (unfortunately, the configurable version is radically\ndifferent from the original hard-coded version, so I can't say this\nhas had much testing yet)\n\nThe definition of the allowed regular expression in the docs\nprobably needs some refinement. Basically, they have to also be REs\nthat can be concatenated with the \"|\" character, which is not true\nof REs that begin with the *** flavor selector (which I had not\nheard of before rereading `man re_syntax` just now) or (?xyz)\nembedded options. Or maybe there's an efficient alternate approach\nto scanning for the next non-overlapping match among several\npatterns that doesn't involve concatenating the patterns.\n\nI'm not sure about the \"one line\" restriction; at first I thought\nthat everything was fed to 'appendwithlinks' in arbitrary chunks,\nbut not I see that they are mostly logical chunks (and probably only\nthe comment, not the headers or commit descriptors, will have\nanything to linkify). The problem again seems to be how to succinctly\ndescribe what is permitted.\n\nThere are probably better names for the configuration options, too.\n\nSuggestions? Problems? Successes?\n\nJeff\n\n\n Documentation/config.txt | 31 ++++++++++++++++++-\n gitk-git/gitk | 74 +++++++++++++++++++++++++++++++++++++++++++++-\n 2 files changed, 102 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 750c86d..67ed436 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1102,6 +1102,33 @@ All gitcvs variables except for 'gitcvs.usecrlfattr' and\n is one of \"ext\" and \"pserver\") to make them apply only for the given\n access method.\n \n+gitk.linkify..re::\n+\tSpecify a Tcl regular expression (which may not span lines)\n+\tdefining a class of strings to automatically convert to hyperlinks.\n+\tYou must also specify 'gitk.linkify..sub'.\n+\n+gitk.linkify..sub::\n+\tSpecify a substitution that results in the target URL for the\n+\trelated regular expression. Back-references like '\\1' refer\n+\tto capturing groups in the associated regular expression.\n+\tYou must also specify 'gitk.linkify..re'.\n+\n+gitk.browser::\n+\tSpecify the browser that will be used to display the linked\n+\tweb page.\n+\n+For example, to automatically link from Debian-style \"Closes: #nnnn\"\n+message to the Debian BTS,\n+\n+--------\n+ git config gitk.linkify.debian-bts.re 'Closes: #(\\d+)'\n+ git config gitk.linkify.debian-bts.sub 'http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=\\1'\n+--------\n+\n+Regular expressions are as described in re_syntax(n). Replacements\n+are as described in regsub(n). If multiple regular expressions match at\n+the same location, it is undefined which match is used.\n+\n grep.lineNumber::\n \tIf set to true, enable '-n' option by default.\n \n@@ -1901,5 +1928,5 @@ user.signingkey::\n \n web.browser::\n \tSpecify a web browser that may be used by some commands.\n-\tCurrently only linkgit:git-instaweb[1] and linkgit:git-help[1]\n-\tmay use it.\n+\tCurrently only linkgit:git-instaweb[1], linkgit:gitk[1],\n+\tand linkgit:git-help[1] may use it.\ndiff --git a/gitk-git/gitk b/gitk-git/gitk\nindex 4cde0c4..a21eea1 100755\n--- a/gitk-git/gitk\n+++ b/gitk-git/gitk\n@@ -6684,7 +6684,7 @@ proc commit_descriptor {p} {\n # append some text to the ctext widget, and make any SHA1 ID\n # that we know about be a clickable link.\n proc appendwithlinks {text tags} {\n- global ctext linknum curview\n+ global ctext linknum curview linkmakers\n \n set start [$ctext index \"end - 1c\"]\n $ctext insert end $text $tags\n@@ -6699,6 +6699,30 @@ proc appendwithlinks {text tags} {\n \tsetlink $linkid link$linknum\n \tincr linknum\n }\n+\n+ if {$linkmakers == {}} return\n+\n+ set link_re {}\n+ foreach {re rep} $linkmakers { lappend link_re $re }\n+ set link_re \"([join $link_re {)|(}])\"\n+\n+ set ee 0\n+ while {[regexp -indices -start $ee -- $link_re $text l]} {\n+\tset s [lindex $l 0]\n+\tset e [lindex $l 1]\n+\tset linktext [string range $text $s $e]\n+\tincr e\n+\tset ee $e\n+\n+\tforeach {re rep} $linkmakers {\n+\t if {![regsub $re $linktext $rep linkurl]} continue\n+\t $ctext tag delete link$linknum\n+\t $ctext tag add link$linknum \"$start + $s c\" \"$start + $e c\"\n+\t seturllink $linkurl link$linknum\n+\t incr linknum\n+\t break\n+\t}\n+ }\n }\n \n proc setlink {id lk} {\n@@ -6726,6 +6750,52 @@ proc setlink {id lk} {\n }\n }\n \n+proc get_link_config {} {\n+ if {[catch {exec git config -z --get-regexp {^gitk\\.linkify\\.}} linkers]} {\n+\treturn {}\n+ }\n+\n+ set linktypes [list]\n+ foreach item [split $linkers \"\\0\"] {\n+\tif {$item == \"\"} continue\n+\tif {![regexp {gitk\\.linkify\\.(\\S+)\\.(re|sub)\\s(.*)} $item _ k t v]} {\n+\t continue\n+\t}\n+\tset linkconfig($t,$k) $v\n+\tif {$t == \"re\"} { lappend linktypes $k }\n+ }\n+\n+ set linkmakers [list]\n+ foreach k $linktypes {\n+\tif {![info exists linkconfig(sub,$k)]} {\n+\t puts stderr \"Warning: link `$k' is missing a substitution string\"\n+\t} elseif {[catch {regexp -inline -- $linkconfig(re,$k) \"\"} err]} {\n+\t puts stderr \"Warning: link `$k': $err\"\n+\t} else {\n+\t lappend linkmakers $linkconfig(re,$k) $linkconfig(sub,$k)\n+\t}\n+\tunset linkconfig(re,$k)\n+\tunset -nocomplain linkconfig(sub,$k)\n+ }\n+ foreach k [array names linkconfig] {\n+\tregexp \"sub,(.*)\" $k _ k\n+\tputs stderr \"Warning: link `$k' is missing a regular expression\"\n+ }\n+ set linkmakers\n+}\n+\n+proc openlink {url} {\n+ exec git web--browse --config=gitk.browser $url &\n+}\n+\n+proc seturllink {url lk} {\n+ global ctext\n+ $ctext tag conf $lk -foreground blue -underline 1\n+ $ctext tag bind $lk <1> [list openlink $url]\n+ $ctext tag bind $lk {linkcursor %W 1}\n+ $ctext tag bind $lk {linkcursor %W -1}\n+}\n+\n proc appendshortlink {id {pre {}} {post {}}} {\n global ctext linknum\n \n@@ -11693,6 +11763,8 @@ if {[tk windowingsystem] eq \"win32\"} {\n focus -force .\n }\n \n+set linkmakers [get_link_config]\n+\n getcommits {}\n \n # Local variables:\n-- \n1.7.0.4\n"},{"id":"175684","messageId":"4E7467B7.1090201@gmail.com","threadId":"28411","inReplyTo":"20110917022903.GA2445@unpythonic.net","subject":"Re: [RFC/PATCH] Configurable hyperlinking in gitk","fromName":"Chris Packham","fromEmail":"judge.packham@gmail.com","sentAt":"2011-09-17T09:26:15Z","receivedAt":"2011-09-17T09:26:15Z","isPatch":true,"sender":{"key":"judge.packham@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155667?v=4"},"body":"Hi,\n\nOn 17/09/11 14:29, Jeff Epler wrote:\n> Some time ago I hardcoded this into gitk for $DAY_JOB and find it very\n> useful. I made it configurable in the hopes that it might be adopted\n> upstream. (unfortunately, the configurable version is radically\n> different from the original hard-coded version, so I can't say this\n> has had much testing yet)\n\nThis is definitely something folks at my $dayjob would be interested in.\nWe've already done some customisation of gitweb to do something similar.\nI'm not actually sure what the changes where or how configurable they\nare. I'll see if I can dig them out on Monday someone else might want to\npolish them into something suitable (I might do it myself if I get some\ntuits).\n\n> The definition of the allowed regular expression in the docs\n> probably needs some refinement. Basically, they have to also be REs\n> that can be concatenated with the \"|\" character, which is not true\n> of REs that begin with the *** flavor selector (which I had not\n> heard of before rereading `man re_syntax` just now) or (?xyz)\n> embedded options. Or maybe there's an efficient alternate approach\n> to scanning for the next non-overlapping match among several\n> patterns that doesn't involve concatenating the patterns.\n> \n> I'm not sure about the \"one line\" restriction; at first I thought\n> that everything was fed to 'appendwithlinks' in arbitrary chunks,\n> but not I see that they are mostly logical chunks (and probably only\n> the comment, not the headers or commit descriptors, will have\n> anything to linkify). The problem again seems to be how to succinctly\n> describe what is permitted.\n\nFor my use case the one line restriction is fine. We tend to put the bug\nnumber in the headline anyway.\n\nSometimes when a commit fixes multiple bugs we put all the bug numbers\nin separated by commas. I don't know Tcl well enough to tell if your\ncode supports that or not.\n\n> There are probably better names for the configuration options, too.\n\nIt'd be nice if the config variables weren't gitk specific. .re and .sub\ncould be applied to gitweb and maybe other git viewers outside of\ngig.git might decide to use them. My bikeshedding suggestion would be to\njust drop the gitk prefix and have linkify.re and linkify.sub.\n\n> Suggestions? Problems? Successes?\n\nRe-compiling now. I won't be able to actually test it properly until I'm\nback in the office but I can at least check that the links are generated.\n"},{"id":"175685","messageId":"4E746FE7.4060608@gmail.com","threadId":"28411","inReplyTo":"4E7467B7.1090201@gmail.com","subject":"Re: [RFC/PATCH] Configurable hyperlinking in gitk","fromName":"Chris Packham","fromEmail":"judge.packham@gmail.com","sentAt":"2011-09-17T10:01:11Z","receivedAt":"2011-09-17T10:01:11Z","isPatch":true,"sender":{"key":"judge.packham@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155667?v=4"},"body":"On 17/09/11 21:26, Chris Packham wrote:\n> Hi,\n> \n> On 17/09/11 14:29, Jeff Epler wrote:\n>> Some time ago I hardcoded this into gitk for $DAY_JOB and find it very\n>> useful. I made it configurable in the hopes that it might be adopted\n>> upstream. (unfortunately, the configurable version is radically\n>> different from the original hard-coded version, so I can't say this\n>> has had much testing yet)\n> \n> This is definitely something folks at my $dayjob would be interested in.\n> We've already done some customisation of gitweb to do something similar.\n> I'm not actually sure what the changes where or how configurable they\n> are. I'll see if I can dig them out on Monday someone else might want to\n> polish them into something suitable (I might do it myself if I get some\n> tuits).\n> \n>> The definition of the allowed regular expression in the docs\n>> probably needs some refinement. Basically, they have to also be REs\n>> that can be concatenated with the \"|\" character, which is not true\n>> of REs that begin with the *** flavor selector (which I had not\n>> heard of before rereading `man re_syntax` just now) or (?xyz)\n>> embedded options. Or maybe there's an efficient alternate approach\n>> to scanning for the next non-overlapping match among several\n>> patterns that doesn't involve concatenating the patterns.\n>>\n>> I'm not sure about the \"one line\" restriction; at first I thought\n>> that everything was fed to 'appendwithlinks' in arbitrary chunks,\n>> but not I see that they are mostly logical chunks (and probably only\n>> the comment, not the headers or commit descriptors, will have\n>> anything to linkify). The problem again seems to be how to succinctly\n>> describe what is permitted.\n> \n> For my use case the one line restriction is fine. We tend to put the bug\n> number in the headline anyway.\n> \n> Sometimes when a commit fixes multiple bugs we put all the bug numbers\n> in separated by commas. I don't know Tcl well enough to tell if your\n> code supports that or not.\n> \n>> There are probably better names for the configuration options, too.\n> \n> It'd be nice if the config variables weren't gitk specific. .re and .sub\n> could be applied to gitweb and maybe other git viewers outside of\n> gig.git might decide to use them. My bikeshedding suggestion would be to\n> just drop the gitk prefix and have linkify.re and linkify.sub.\n\nThat should be linkify..re and linkify..sub\n\n>> Suggestions? Problems? Successes?\n> \n> Re-compiling now. I won't be able to actually test it properly until I'm\n> back in the office but I can at least check that the links are generated.\n\nSlight complication. The URL of our bug tracker has an ampersand '&' in\nit. Tcl's substitution does what one might expect and puts the matched\ntext where the '&' is. I've tried using url friendly %26 but something\neats the %. I've also tried backslashes to no avail.\n\nTo answer my own question since I started writing this email I've found\nthat using %% works (only the first one gets eaten). Not sure if that's\nexpected behaviour or not (printf escaping maybe?).\n\nAlso since I've been playing around I've tried a commit with multiple\nbug numbers on one line and that works as expected.\n\nThanks\nChris\n"},{"id":"175693","messageId":"20110917134527.GA28463@unpythonic.net","threadId":"28411","inReplyTo":"4E7467B7.1090201@gmail.com","subject":"Re: [RFC/PATCH] Configurable hyperlinking in gitk","fromName":"Jeff Epler","fromEmail":"jepler@unpythonic.net","sentAt":"2011-09-17T13:45:28Z","receivedAt":"2011-09-17T13:45:28Z","isPatch":true,"sender":{"key":"jepler@unpythonic.net","avatar":"https://avatars.githubusercontent.com/u/1517291?v=4"},"body":"> > There are probably better names for the configuration options, too.\n> \n> It'd be nice if the config variables weren't gitk specific. .re and .sub\n> could be applied to gitweb and maybe other git viewers outside of\n> gig.git might decide to use them. My bikeshedding suggestion would be to\n> just drop the gitk prefix and have linkify.re and linkify.sub.\n\nThis seems like a reasonable idea, though since the implementation\nlanguages of gitk and gitweb are different it means some REs might get\ndifferent interpretations in the different programs.\n\n> Sometimes when a commit fixes multiple bugs we put all the bug numbers\n> in separated by commas. I don't know Tcl well enough to tell if your\n> code supports that or not.\n\nMultiple matches per line are OK, but they must be non-overlapping.\n\nLooking at the actual practice in Debian changelogs, I see that they do\nthis:\n evince/changelog.Debian.gz: (Closes: #388368, #396467, #405130)\nso my original example would only linkify \"Closes: #388638\". But a\nrevised pattern of #(\\d+) would linkify \"#388368\", \"#396467\" and \"#405130\".\n(but risk a few more \"false positive\" links). I should revise my\nexample accordingly.\n\nAs for the problems with your substitutions, \"&\" is special in a tcl\nregsub (it stands for the whole matched string, like \\0), so you'd want\nto use a substitution like\n git config gitk.linkify.debian-bts.sub \\\n 'http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=\\1\\&foo=bar'\nThe problem with \"%\" has to do with Tk's event substitution and it's a\nbug that this happens; I should manually double the % at the proper\npoint.\n\nThis revised patch fixes the problem with % in substitutions and changes\nthe suggested RE for matching debian bts items, but it does not rename\nthe configuration options.\n\n-- >8 --\nMany projects use project-specific notations in changelogs to refer\nto bug trackers and the like. One example is the \"Closes: #12345\"\nnotation used in Debian.\n\nMake gitk configurable so that arbitrary strings can be turned into\nclickable links that are opened in a web browser.\n---\n Documentation/config.txt | 31 ++++++++++++++++++-\n gitk-git/gitk | 75 +++++++++++++++++++++++++++++++++++++++++++++-\n 2 files changed, 103 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex ae9913b..13e8aa6 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1064,6 +1064,33 @@ All gitcvs variables except for 'gitcvs.usecrlfattr' and\n is one of \"ext\" and \"pserver\") to make them apply only for the given\n access method.\n \n+gitk.linkify..re::\n+\tSpecify a Tcl regular expression (which may not span lines)\n+\tdefining a class of strings to automatically convert to hyperlinks.\n+\tYou must also specify 'gitk.linkify..sub'.\n+\n+gitk.linkify..sub::\n+\tSpecify a substitution that results in the target URL for the\n+\trelated regular expression. Back-references like '\\1' refer\n+\tto capturing groups in the associated regular expression.\n+\tYou must also specify 'gitk.linkify..re'.\n+\n+gitk.browser::\n+\tSpecify the browser that will be used to display the linked\n+\tweb page.\n+\n+For example, to automatically link from Debian-style \"Closes: #nnnn\"\n+message to the Debian BTS,\n+\n+--------\n+ git config gitk.linkify.debian-bts.re '#(\\d+)\\M'\n+ git config gitk.linkify.debian-bts.sub 'http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=\\1'\n+--------\n+\n+Regular expressions are as described in re_syntax(n). Replacements\n+are as described in regsub(n). If multiple regular expressions match at\n+the same location, it is undefined which match is used.\n+\n grep.lineNumber::\n \tIf set to true, enable '-n' option by default.\n \n@@ -1870,5 +1897,5 @@ user.signingkey::\n \n web.browser::\n \tSpecify a web browser that may be used by some commands.\n-\tCurrently only linkgit:git-instaweb[1] and linkgit:git-help[1]\n-\tmay use it.\n+\tCurrently only linkgit:git-instaweb[1], linkgit:gitk[1],\n+\tand linkgit:git-help[1] may use it.\ndiff --git a/gitk-git/gitk b/gitk-git/gitk\nindex 4cde0c4..5532869 100755\n--- a/gitk-git/gitk\n+++ b/gitk-git/gitk\n@@ -6684,7 +6684,7 @@ proc commit_descriptor {p} {\n # append some text to the ctext widget, and make any SHA1 ID\n # that we know about be a clickable link.\n proc appendwithlinks {text tags} {\n- global ctext linknum curview\n+ global ctext linknum curview linkmakers\n \n set start [$ctext index \"end - 1c\"]\n $ctext insert end $text $tags\n@@ -6699,6 +6699,30 @@ proc appendwithlinks {text tags} {\n \tsetlink $linkid link$linknum\n \tincr linknum\n }\n+\n+ if {$linkmakers == {}} return\n+\n+ set link_re {}\n+ foreach {re rep} $linkmakers { lappend link_re $re }\n+ set link_re \"([join $link_re {)|(}])\"\n+\n+ set ee 0\n+ while {[regexp -indices -start $ee -- $link_re $text l]} {\n+\tset s [lindex $l 0]\n+\tset e [lindex $l 1]\n+\tset linktext [string range $text $s $e]\n+\tincr e\n+\tset ee $e\n+\n+\tforeach {re rep} $linkmakers {\n+\t if {![regsub $re $linktext $rep linkurl]} continue\n+\t $ctext tag delete link$linknum\n+\t $ctext tag add link$linknum \"$start + $s c\" \"$start + $e c\"\n+\t seturllink $linkurl link$linknum\n+\t incr linknum\n+\t break\n+\t}\n+ }\n }\n \n proc setlink {id lk} {\n@@ -6726,6 +6750,53 @@ proc setlink {id lk} {\n }\n }\n \n+proc get_link_config {} {\n+ if {[catch {exec git config -z --get-regexp {^gitk\\.linkify\\.}} linkers]} {\n+\treturn {}\n+ }\n+\n+ set linktypes [list]\n+ foreach item [split $linkers \"\\0\"] {\n+\tif {$item == \"\"} continue\n+\tif {![regexp {gitk\\.linkify\\.(\\S+)\\.(re|sub)\\s(.*)} $item _ k t v]} {\n+\t continue\n+\t}\n+\tset linkconfig($t,$k) $v\n+\tif {$t == \"re\"} { lappend linktypes $k }\n+ }\n+\n+ set linkmakers [list]\n+ foreach k $linktypes {\n+\tif {![info exists linkconfig(sub,$k)]} {\n+\t puts stderr \"Warning: link `$k' is missing a substitution string\"\n+\t} elseif {[catch {regexp -inline -- $linkconfig(re,$k) \"\"} err]} {\n+\t puts stderr \"Warning: link `$k': $err\"\n+\t} else {\n+\t lappend linkmakers $linkconfig(re,$k) $linkconfig(sub,$k)\n+\t}\n+\tunset linkconfig(re,$k)\n+\tunset -nocomplain linkconfig(sub,$k)\n+ }\n+ foreach k [array names linkconfig] {\n+\tregexp \"sub,(.*)\" $k _ k\n+\tputs stderr \"Warning: link `$k' is missing a regular expression\"\n+ }\n+ set linkmakers\n+}\n+\n+proc openlink {url} {\n+ exec git web--browse --config=gitk.browser $url &\n+}\n+\n+proc seturllink {url lk} {\n+ set qurl [string map {% %%} $url]\n+ global ctext\n+ $ctext tag conf $lk -foreground blue -underline 1\n+ $ctext tag bind $lk <1> [list openlink $qurl]\n+ $ctext tag bind $lk {linkcursor %W 1}\n+ $ctext tag bind $lk {linkcursor %W -1}\n+}\n+\n proc appendshortlink {id {pre {}} {post {}}} {\n global ctext linknum\n \n@@ -11693,6 +11764,8 @@ if {[tk windowingsystem] eq \"win32\"} {\n focus -force .\n }\n \n+set linkmakers [get_link_config]\n+\n getcommits {}\n \n # Local variables:\n-- \n1.7.2.5\n"},{"id":"175696","messageId":"4E752E32.2010208@gmail.com","threadId":"28411","inReplyTo":"20110917134527.GA28463@unpythonic.net","subject":"Re: [RFC/PATCH] Configurable hyperlinking in gitk","fromName":"Chris Packham","fromEmail":"judge.packham@gmail.com","sentAt":"2011-09-17T23:33:06Z","receivedAt":"2011-09-17T23:33:06Z","isPatch":true,"sender":{"key":"judge.packham@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155667?v=4"},"body":"On 18/09/11 01:45, Jeff Epler wrote:\n>>> There are probably better names for the configuration options, too.\n>>\n>> It'd be nice if the config variables weren't gitk specific. .re and .sub\n>> could be applied to gitweb and maybe other git viewers outside of\n>> gig.git might decide to use them. My bikeshedding suggestion would be to\n>> just drop the gitk prefix and have linkify.re and linkify.sub.\n> \n> This seems like a reasonable idea, though since the implementation\n> languages of gitk and gitweb are different it means some REs might get\n> different interpretations in the different programs.\n> \n>> Sometimes when a commit fixes multiple bugs we put all the bug numbers\n>> in separated by commas. I don't know Tcl well enough to tell if your\n>> code supports that or not.\n> \n> Multiple matches per line are OK, but they must be non-overlapping.\n> \n> Looking at the actual practice in Debian changelogs, I see that they do\n> this:\n> evince/changelog.Debian.gz: (Closes: #388368, #396467, #405130)\n> so my original example would only linkify \"Closes: #388638\". But a\n> revised pattern of #(\\d+) would linkify \"#388368\", \"#396467\" and \"#405130\".\n> (but risk a few more \"false positive\" links). I should revise my\n> example accordingly.\n> \n> As for the problems with your substitutions, \"&\" is special in a tcl\n> regsub (it stands for the whole matched string, like \\0), so you'd want\n> to use a substitution like\n> git config gitk.linkify.debian-bts.sub \\\n> 'http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=\\1\\&foo=bar'\n\nHmm no joy with \\&. Seems to upset the invocation of git web-browse\n\n git config gitk.linkify.bugtracker.sub \\\n 'https://internalhost/code\\&stuff/bugs.php?id=\\1'\n\n gitk\n /home/chrisp/libexec/git-core/git-web--browse: line 167:\nstuff/bugs.php?id=bug123: No such file or directory\n fatal: 'web--browse' appears to be a git command, but we were not\n able to execute it. Maybe git-web--browse is broken?\n\nUsing the following works as expected with no error with your updated patch.\n\n git config gitk.linkify.bugtracker.sub \\\n 'https://internalhost/code%26stuff/bugs.php?id=\\1'\n\n> The problem with \"%\" has to do with Tk's event substitution and it's a\n> bug that this happens; I should manually double the % at the proper\n> point.\n> \n"},{"id":"175698","messageId":"4E753BB9.7030804@gmail.com","threadId":"28411","inReplyTo":"4E752E32.2010208@gmail.com","subject":"git web--browse error handling URL with & in it (Was Re: [RFC/PATCH] Configurable hyperlinking in gitk)","fromName":"Chris Packham","fromEmail":"judge.packham@gmail.com","sentAt":"2011-09-18T00:30:49Z","receivedAt":"2011-09-18T00:30:49Z","isPatch":true,"sender":{"key":"judge.packham@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155667?v=4"},"body":"On 18/09/11 11:33, Chris Packham wrote:\n> On 18/09/11 01:45, Jeff Epler wrote:\n>>>> There are probably better names for the configuration options, too.\n>>>\n>>> It'd be nice if the config variables weren't gitk specific. .re and .sub\n>>> could be applied to gitweb and maybe other git viewers outside of\n>>> gig.git might decide to use them. My bikeshedding suggestion would be to\n>>> just drop the gitk prefix and have linkify.re and linkify.sub.\n>>\n>> This seems like a reasonable idea, though since the implementation\n>> languages of gitk and gitweb are different it means some REs might get\n>> different interpretations in the different programs.\n>>\n>>> Sometimes when a commit fixes multiple bugs we put all the bug numbers\n>>> in separated by commas. I don't know Tcl well enough to tell if your\n>>> code supports that or not.\n>>\n>> Multiple matches per line are OK, but they must be non-overlapping.\n>>\n>> Looking at the actual practice in Debian changelogs, I see that they do\n>> this:\n>> evince/changelog.Debian.gz: (Closes: #388368, #396467, #405130)\n>> so my original example would only linkify \"Closes: #388638\". But a\n>> revised pattern of #(\\d+) would linkify \"#388368\", \"#396467\" and \"#405130\".\n>> (but risk a few more \"false positive\" links). I should revise my\n>> example accordingly.\n>>\n>> As for the problems with your substitutions, \"&\" is special in a tcl\n>> regsub (it stands for the whole matched string, like \\0), so you'd want\n>> to use a substitution like\n>> git config gitk.linkify.debian-bts.sub \\\n>> 'http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=\\1\\&foo=bar'\n> \n> Hmm no joy with \\&. Seems to upset the invocation of git web-browse\n> \n> git config gitk.linkify.bugtracker.sub \\\n> 'https://internalhost/code\\&stuff/bugs.php?id=\\1'\n> \n> gitk\n> /home/chrisp/libexec/git-core/git-web--browse: line 167:\n> stuff/bugs.php?id=bug123: No such file or directory\n> fatal: 'web--browse' appears to be a git command, but we were not\n> able to execute it. Maybe git-web--browse is broken?\n\nThis is probably a issue with git web--browse and nothing to do with\nyour changes.\n\nSure enough this works fine\n\n git web--browse --browser=firefox \\\n https://internalhost/code\\&stuff/bugs.php?id=foo\n\nWhile this doesn't\n\n git web--browse https://internalhost/code\\&stuff/bugs.php?id=foo\n\n/home/chrisp/libexec/git-core/git-web--browse: line 167:\nstuff/bugs.php?id=foo: No such file or directory\nfatal: 'web--browse' appears to be a git command, but we were not\nable to execute it. Maybe git-web--browse is broken?\n\nNeither does this\n\n git web--browse --browser=konqueror \\\n https://internalhost/code\\&stuff/bugs.php?id=foo\n\nA little bit more info that might help diagnose the issue - I'm running\nopenSUSE 11.4 (kde 4.6) which ships with firefox set as the default web\nbrowser so 'kfmclient newTab http://www.example.com' actually opens firefox.\n\nHowever trying kfmclient with my funny URL still works\n\n kfmclient newTab https://internalhost/code\\&stuff/bugs.php?id=foo\n\nI'm a little stumped as to what is going wrong in git web--browse.\n"},{"id":"175699","messageId":"4E753C04.1070202@gmail.com","threadId":"28411","inReplyTo":"4E753BB9.7030804@gmail.com","subject":"Re: git web--browse error handling URL with & in it (Was Re: [RFC/PATCH] Configurable hyperlinking in gitk)","fromName":"Chris Packham","fromEmail":"judge.packham@gmail.com","sentAt":"2011-09-18T00:32:04Z","receivedAt":"2011-09-18T00:32:04Z","isPatch":true,"sender":{"key":"judge.packham@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155667?v=4"},"body":"On 18/09/11 12:30, Chris Packham wrote:\n> On 18/09/11 11:33, Chris Packham wrote:\n>> On 18/09/11 01:45, Jeff Epler wrote:\n>>>>> There are probably better names for the configuration options, too.\n>>>>\n>>>> It'd be nice if the config variables weren't gitk specific. .re and .sub\n>>>> could be applied to gitweb and maybe other git viewers outside of\n>>>> gig.git might decide to use them. My bikeshedding suggestion would be to\n>>>> just drop the gitk prefix and have linkify.re and linkify.sub.\n>>>\n>>> This seems like a reasonable idea, though since the implementation\n>>> languages of gitk and gitweb are different it means some REs might get\n>>> different interpretations in the different programs.\n>>>\n>>>> Sometimes when a commit fixes multiple bugs we put all the bug numbers\n>>>> in separated by commas. I don't know Tcl well enough to tell if your\n>>>> code supports that or not.\n>>>\n>>> Multiple matches per line are OK, but they must be non-overlapping.\n>>>\n>>> Looking at the actual practice in Debian changelogs, I see that they do\n>>> this:\n>>> evince/changelog.Debian.gz: (Closes: #388368, #396467, #405130)\n>>> so my original example would only linkify \"Closes: #388638\". But a\n>>> revised pattern of #(\\d+) would linkify \"#388368\", \"#396467\" and \"#405130\".\n>>> (but risk a few more \"false positive\" links). I should revise my\n>>> example accordingly.\n>>>\n>>> As for the problems with your substitutions, \"&\" is special in a tcl\n>>> regsub (it stands for the whole matched string, like \\0), so you'd want\n>>> to use a substitution like\n>>> git config gitk.linkify.debian-bts.sub \\\n>>> 'http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=\\1\\&foo=bar'\n>>\n>> Hmm no joy with \\&. Seems to upset the invocation of git web-browse\n>>\n>> git config gitk.linkify.bugtracker.sub \\\n>> 'https://internalhost/code\\&stuff/bugs.php?id=\\1'\n>>\n>> gitk\n>> /home/chrisp/libexec/git-core/git-web--browse: line 167:\n>> stuff/bugs.php?id=bug123: No such file or directory\n>> fatal: 'web--browse' appears to be a git command, but we were not\n>> able to execute it. Maybe git-web--browse is broken?\n> \n> This is probably a issue with git web--browse and nothing to do with\n> your changes.\n> \n> Sure enough this works fine\n> \n> git web--browse --browser=firefox \\\n> https://internalhost/code\\&stuff/bugs.php?id=foo\n> \n> While this doesn't\n> \n> git web--browse https://internalhost/code\\&stuff/bugs.php?id=foo\n> \n> /home/chrisp/libexec/git-core/git-web--browse: line 167:\n> stuff/bugs.php?id=foo: No such file or directory\n> fatal: 'web--browse' appears to be a git command, but we were not\n> able to execute it. Maybe git-web--browse is broken?\n> \n> Neither does this\n> \n> git web--browse --browser=konqueror \\\n> https://internalhost/code\\&stuff/bugs.php?id=foo\n> \n> A little bit more info that might help diagnose the issue - I'm running\n> openSUSE 11.4 (kde 4.6) which ships with firefox set as the default web\n> browser so 'kfmclient newTab http://www.example.com' actually opens firefox.\n> \n> However trying kfmclient with my funny URL still works\n> \n> kfmclient newTab https://internalhost/code\\&stuff/bugs.php?id=foo\n> \n> I'm a little stumped as to what is going wrong in git web--browse.\n> \n\nUpdate: it's the call to eval that causes the problem\n\n eval kfmclient newTab https://internalhost/code\\&stuff/bugs.php?id=foo\n [1] 14728\n bash: stuff/bugs.php?id=foo: No such file or directory\n"},{"id":"175709","messageId":"20110918032933.GA17977@sigill.intra.peff.net","threadId":"28411","inReplyTo":"4E753C04.1070202@gmail.com","subject":"Re: git web--browse error handling URL with & in it (Was Re: [RFC/PATCH] Configurable hyperlinking in gitk)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-09-18T03:29:34Z","receivedAt":"2011-09-18T03:29:34Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Sep 18, 2011 at 12:32:04PM +1200, Chris Packham wrote:\n\n> Update: it's the call to eval that causes the problem\n> \n> eval kfmclient newTab https://internalhost/code\\&stuff/bugs.php?id=foo\n> [1] 14728\n> bash: stuff/bugs.php?id=foo: No such file or directory\n\nHmm. The offending lines look like:\n\n eval \"$browser_path\" \"$@\" &\n\nNormally in git we treat user-configured commands as shell snippets,\nmeaning the user is responsible for any quoting. But in this script, we\nseem to run:\n\n type \"$browser_path\"\n\nseveral times. Which implies that \"$browser_path\" must be the actual\nexecutable. In which case, I would think that:\n\n \"$browser_path\" \"$@\" &\n\nwould be the right thing. And indeed, that is what the firefox arm of\nthe case statement does. But chrome, konqueror, and others use eval.\n\nUnrelated, but it also looks like $browser_path is used unquoted in the\nfirefox case (see inside the vers=$(...)).\n\n-Peff\n"},{"id":"175712","messageId":"1316341224-4359-1-git-send-email-judge.packham@gmail.com","threadId":"28411","inReplyTo":"20110918032933.GA17977@sigill.intra.peff.net","subject":"[PATCH] git-web--browse: invoke kfmclient directly","fromName":"Chris Packham","fromEmail":"judge.packham@gmail.com","sentAt":"2011-09-18T10:20:24Z","receivedAt":"2011-09-18T10:20:24Z","isPatch":true,"sender":{"key":"judge.packham@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155667?v=4"},"body":"Instead of using eval which causes problems when a URL contains an\nappropriately escaped ampersand (\\&).\n\nCc: peff@peff.net\nCc: chriscool@tuxfamily.org\nCc: jepler@unpythonic.net\nSigned-off-by: Chris Packham \n---\n> Which implies that \"$browser_path\" must be the actual\n> executable. In which case, I would think that:\n>\n> \"$browser_path\" \"$@\" &\n>\n> would be the right thing. And indeed, that is what the firefox arm of\n> the case statement does. But chrome, konqueror, and others use eval.\n\nSo here is my attempt at a fix for kfmclient.\n\nFor what it's worth I've included a testcase that detects my problem. I'm not\nsure if the testcase is really worth it because the test library suppresses X\napplications and even if it didn't the testcase is fairly trivial and might\njust annoy people by opening web-browsers (and it snaps up the last t99xx\nprefix). \n\n git-web--browse.sh | 4 ++--\n t/t9901-git-web--browse.sh | 43 +++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 45 insertions(+), 2 deletions(-)\n create mode 100755 t/t9901-git-web--browse.sh\n\ndiff --git a/git-web--browse.sh b/git-web--browse.sh\nindex e9de241..1164a22 100755\n--- a/git-web--browse.sh\n+++ b/git-web--browse.sh\n@@ -164,10 +164,10 @@ konqueror)\n \t\t# It's simpler to use kfmclient to open a new tab in konqueror.\n \t\tbrowser_path=\"$(echo \"$browser_path\" | sed -e 's/konqueror$/kfmclient/')\"\n \t\ttype \"$browser_path\" > /dev/null 2>&1 || die \"No '$browser_path' found.\"\n-\t\teval \"$browser_path\" newTab \"$@\"\n+\t\t\"$browser_path\" newTab \"$@\" &\n \t\t;;\n \tkfmclient)\n-\t\teval \"$browser_path\" newTab \"$@\"\n+\t\t\"$browser_path\" newTab \"$@\" &\n \t\t;;\n \t*)\n \t\t\"$browser_path\" \"$@\" &\ndiff --git a/t/t9901-git-web--browse.sh b/t/t9901-git-web--browse.sh\nnew file mode 100755\nindex 0000000..7ed38a0\n--- /dev/null\n+++ b/t/t9901-git-web--browse.sh\n@@ -0,0 +1,43 @@\n+#!/bin/sh\n+#\n+# Copyright (c) 2011 Chris Packham\n+#\n+\n+test_description='git web--browse basic tests\n+\n+This test checks that git web--browse can handle various valid URLs with\n+the supported browsers that are installed on the host system.'\n+\n+. ./test-lib.sh\n+\n+test -x /usr/bin/firefox && test_set_prereq FIREFOX\n+test -x /usr/bin/konqueror && test_set_prereq KONQUEROR\n+test -x /usr/bin/google-chrome && test_set_prereq CHROME\n+test -x /usr/bin/opera && test_set_prereq OPERA\n+\n+test_expect_success \\\n+\t'accepts a URL with an ampersand in it (default)' '\n+ git web--browse http://example.com/foo\\&bar/\n+'\n+\n+test_expect_success FIREFOX \\\n+\t'accepts a URL with an ampersand in it (firefox)' '\n+ git web--browse --browser=firefox http://example.com/foo\\&bar/\n+'\n+\n+test_expect_success KONQUEROR \\\n+\t'accepts a URL with an ampersand in it (konqueror)' '\n+ git web--browse --browser=konqueror http://example.com/foo\\&bar/\n+'\n+\n+test_expect_success OPERA \\\n+\t'accepts a URL with an ampersand in it (opera)' '\n+ git web--browse --browser=opera http://example.com/foo\\&bar/\n+'\n+\n+test_expect_success CHROME \\\n+\t'accepts a URL with an ampersand in it (chrome)' '\n+ git web--browse --browser=google-chrome http://example.com/foo\\&bar/\n+'\n+\n+test_done\n-- \n1.7.7.rc1.3.g5593.dirty\n"},{"id":"175720","messageId":"201109181646.36821.chriscool@tuxfamily.org","threadId":"28411","inReplyTo":"20110918032933.GA17977@sigill.intra.peff.net","subject":"Re: git web--browse error handling URL with & in it (Was Re: [RFC/PATCH] Configurable hyperlinking in gitk)","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2011-09-18T14:46:35Z","receivedAt":"2011-09-18T14:46:35Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Hi,\n\nOn Sunday 18 September 2011 05:29:34 Jeff King wrote:\n> On Sun, Sep 18, 2011 at 12:32:04PM +1200, Chris Packham wrote:\n> > Update: it's the call to eval that causes the problem\n> > \n> > eval kfmclient newTab https://internalhost/code\\&stuff/bugs.php?id=foo\n> > [1] 14728\n> > bash: stuff/bugs.php?id=foo: No such file or directory\n> \n> Hmm. The offending lines look like:\n> \n> eval \"$browser_path\" \"$@\" &\n> \n> Normally in git we treat user-configured commands as shell snippets,\n> meaning the user is responsible for any quoting. But in this script, we\n> seem to run:\n> \n> type \"$browser_path\"\n> \n> several times. Which implies that \"$browser_path\" must be the actual\n> executable. In which case, I would think that:\n> \n> \"$browser_path\" \"$@\" &\n> \n> would be the right thing. And indeed, that is what the firefox arm of\n> the case statement does. But chrome, konqueror, and others use eval.\n\nYeah, I don't remember why I sometimes used 'eval \"$browser_path\" \"$@\"' when I \nwrote this code. Sorry!\n \n> Unrelated, but it also looks like $browser_path is used unquoted in the\n> firefox case (see inside the vers=$(...)).\n\nThanks,\nChristian.\n"},{"id":"175726","messageId":"20110918183846.GA31176@sigill.intra.peff.net","threadId":"28411","inReplyTo":"1316341224-4359-1-git-send-email-judge.packham@gmail.com","subject":"Re: [PATCH] git-web--browse: invoke kfmclient directly","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-09-18T18:38:47Z","receivedAt":"2011-09-18T18:38:47Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Sep 18, 2011 at 10:20:24PM +1200, Chris Packham wrote:\n\n> Instead of using eval which causes problems when a URL contains an\n> appropriately escaped ampersand (\\&).\n\nI think this probably should just remove all of the evals. I don't see\nhow any of them is doing any good, and they're actively breaking URLs\nthat need quoting.\n\nHmm. Actually, the one for custom browser commands might need it,\nbecause that one is expected to be a shell snippet. I suspect the\nsimplest thing is to do something like:\n\n eval \"$browser_cmd \\\"\\$@\\\"\"\n\nThe other option would be to actually shell-quote each argument, which\nis a pain to do in the shell (but is what C git does).\n\n> For what it's worth I've included a testcase that detects my problem. I'm not\n> sure if the testcase is really worth it because the test library suppresses X\n> applications and even if it didn't the testcase is fairly trivial and might\n> just annoy people by opening web-browsers (and it snaps up the last t99xx\n> prefix).\n\nIck, yeah. Actually starting real browsers interacts too much with the\nworld outside of the test scripts. The results will be annoying (new\nbrowser windows) and cause non-deterministic test results.\n\nIf you want to make a test, I think you would do better with something\nlike:\n\n echo someurl_with_&_in_it >expect &&\n git config browser.custom.cmd echo &&\n git web--browse --browser=custom someurl_with_&_in_it >actual &&\n test_cmp expect actual\n\nThat won't test that we are invoking kfmclient correctly, obviously, but\nyou can confirm at least that URLs are making it through to the browser\nscript intact.\n\n-Peff\n"},{"id":"175729","messageId":"m3hb49sn26.fsf@localhost.localdomain","threadId":"28411","inReplyTo":"4E7467B7.1090201@gmail.com","subject":"Re: [RFC/PATCH] Configurable hyperlinking in gitk","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2011-09-18T18:50:30Z","receivedAt":"2011-09-18T18:50:30Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Chris Packham writes:\n> On 17/09/11 14:29, Jeff Epler wrote:\n\n> > Some time ago I hardcoded this into gitk for $DAY_JOB and find it very\n> > useful. I made it configurable in the hopes that it might be adopted\n> > upstream. (unfortunately, the configurable version is radically\n> > different from the original hard-coded version, so I can't say this\n> > has had much testing yet)\n> \n> This is definitely something folks at my $dayjob would be interested in.\n> We've already done some customisation of gitweb to do something similar.\n> I'm not actually sure what the changes where or how configurable they\n> are. I'll see if I can dig them out on Monday someone else might want to\n> polish them into something suitable (I might do it myself if I get some\n> tuits).\n\nThat would be nice. So called \"committags\" support was long planned\nfor gitweb, and even some preliminary work exists...\n \n> > There are probably better names for the configuration options, too.\n> \n> It'd be nice if the config variables weren't gitk specific. .re and .sub\n> could be applied to gitweb and maybe other git viewers outside of\n> gig.git might decide to use them. My bikeshedding suggestion would be to\n> just drop the gitk prefix and have linkify.re and linkify.sub.\n\nPerhaps more descriptive name, i.e.\n\n linkify..regexp\n linkify..subst\n\nwould be better?\n\nI guess that regexp is an extended regular expression, isn't it?\n\n-- \nJakub Narębski\n"},{"id":"175778","messageId":"1316424415-11156-1-git-send-email-judge.packham@gmail.com","threadId":"28411","inReplyTo":"20110918183846.GA31176@sigill.intra.peff.net","subject":"[RFC/PATCHv2] git-web--browse: avoid the use of eval","fromName":"Chris Packham","fromEmail":"judge.packham@gmail.com","sentAt":"2011-09-19T09:26:55Z","receivedAt":"2011-09-19T09:26:55Z","isPatch":false,"sender":{"key":"judge.packham@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155667?v=4"},"body":"Using eval causes problems when the URL contains an appropriately\nescaped ampersand (\\&). Dropping eval from the built-in browser\ninvocation avoids the problem.\n\nCc: peff@peff.net\nCc: chriscool@tuxfamily.org\nCc: jepler@unpythonic.net\n\nSigned-off-by: Chris Packham \n\n---\nHere's an updated patch which drops the uses of eval when invoking a\nsupported browser. The default case still uses eval but adds some extra\nquoting which also fixes the problem. I've avoided touching the 'start'\ncase because I don't have access to a windows system to test with.\n\nI've replaced my tests With the test suggested by Peff (should I be\ngiving him credit in the copyright line or something?). I've grabbed\nt9901 but if there is a better set of miscellaneous minor tests that I\nshould be using let me know.\n\n git-web--browse.sh | 10 +++++-----\n t/t9901-git-web--browse.sh | 21 +++++++++++++++++++++\n 2 files changed, 26 insertions(+), 5 deletions(-)\n create mode 100755 t/t9901-git-web--browse.sh\n\ndiff --git a/git-web--browse.sh b/git-web--browse.sh\nindex e9de241..ee05f10 100755\n--- a/git-web--browse.sh\n+++ b/git-web--browse.sh\n@@ -156,7 +156,7 @@ firefox|iceweasel|seamonkey|iceape)\n \t;;\n google-chrome|chrome|chromium|chromium-browser)\n \t# No need to specify newTab. It's default in chromium\n-\teval \"$browser_path\" \"$@\" &\n+\t\"$browser_path\" \"$@\" &\n \t;;\n konqueror)\n \tcase \"$(basename \"$browser_path\")\" in\n@@ -164,10 +164,10 @@ konqueror)\n \t\t# It's simpler to use kfmclient to open a new tab in konqueror.\n \t\tbrowser_path=\"$(echo \"$browser_path\" | sed -e 's/konqueror$/kfmclient/')\"\n \t\ttype \"$browser_path\" > /dev/null 2>&1 || die \"No '$browser_path' found.\"\n-\t\teval \"$browser_path\" newTab \"$@\"\n+\t\t\"$browser_path\" newTab \"$@\" &\n \t\t;;\n \tkfmclient)\n-\t\teval \"$browser_path\" newTab \"$@\"\n+\t\t\"$browser_path\" newTab \"$@\" &\n \t\t;;\n \t*)\n \t\t\"$browser_path\" \"$@\" &\n@@ -175,7 +175,7 @@ konqueror)\n \tesac\n \t;;\n w3m|elinks|links|lynx|open)\n-\teval \"$browser_path\" \"$@\"\n+\t\"$browser_path\" \"$@\"\n \t;;\n start)\n \texec \"$browser_path\" '\"web-browse\"' \"$@\"\n@@ -185,7 +185,7 @@ opera|dillo)\n \t;;\n *)\n \tif test -n \"$browser_cmd\"; then\n-\t\t( eval $browser_cmd \"$@\" )\n+\t\t( eval $browser_cmd \\\"\"$@\"\\\" )\n \tfi\n \t;;\n esac\ndiff --git a/t/t9901-git-web--browse.sh b/t/t9901-git-web--browse.sh\nnew file mode 100755\nindex 0000000..141ed17\n--- /dev/null\n+++ b/t/t9901-git-web--browse.sh\n@@ -0,0 +1,21 @@\n+#!/bin/sh\n+#\n+# Copyright (c) 2011 Chris Packham\n+#\n+\n+test_description='git web--browse basic tests\n+\n+This test checks that git web--browse can handle various valid URLs.'\n+\n+. ./test-lib.sh\n+\n+test_expect_success \\\n+\t'accepts a URL with an ampersand in it' '\n+\techo http://example.com/foo\\&bar/ >expect &&\n+\tgit config browser.custom.cmd echo &&\n+\tgit web--browse --browser=custom \\\n+\t\thttp://example.com/foo\\&bar/ >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_done\n-- \n1.7.7.rc1.4.g47f23.dirty\n"},{"id":"175786","messageId":"4E775A50.4070805@xiplink.com","threadId":"28411","inReplyTo":"4E752E32.2010208@gmail.com","subject":"Re: [RFC/PATCH] Configurable hyperlinking in gitk","fromName":"Marc Branchaud","fromEmail":"marcnarc@xiplink.com","sentAt":"2011-09-19T15:05:52Z","receivedAt":"2011-09-19T15:05:52Z","isPatch":true,"sender":{"key":"marcnarc@xiplink.com","avatar":"https://avatars.githubusercontent.com/u/14980203?v=4"},"body":"On 11-09-17 07:33 PM, Chris Packham wrote:\n> \n> Hmm no joy with \\&. Seems to upset the invocation of git web-browse\n> \n> git config gitk.linkify.bugtracker.sub \\\n> 'https://internalhost/code\\&stuff/bugs.php?id=\\1'\n> \n> gitk\n> /home/chrisp/libexec/git-core/git-web--browse: line 167:\n> stuff/bugs.php?id=bug123: No such file or directory\n> fatal: 'web--browse' appears to be a git command, but we were not\n> able to execute it. Maybe git-web--browse is broken?\n> \n> Using the following works as expected with no error with your updated patch.\n> \n> git config gitk.linkify.bugtracker.sub \\\n> 'https://internalhost/code%26stuff/bugs.php?id=\\1'\n\nJeff: This is great -- thanks!\n\nI still had problems with using an & in the URL, even with the updated patch.\n I had to apply Chris's git-web--browse patch to get it to work.\n\n\t\tM.\n"},{"id":"175792","messageId":"7vvcso9zzi.fsf@alter.siamese.dyndns.org","threadId":"28411","inReplyTo":"20110918183846.GA31176@sigill.intra.peff.net","subject":"Re: [PATCH] git-web--browse: invoke kfmclient directly","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-19T17:57:37Z","receivedAt":"2011-09-19T17:57:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Hmm. Actually, the one for custom browser commands might need it,\n> because that one is expected to be a shell snippet. I suspect the\n> simplest thing is to do something like:\n>\n> eval \"$browser_cmd \\\"\\$@\\\"\"\n\nYeah, I agree, and the dq around $browser_cmd is kind of important, too,\nfor that to work and be readable.\n> If you want to make a test, I think you would do better with something\n> like:\n>\n> echo someurl_with_&_in_it >expect &&\n> git config browser.custom.cmd echo &&\n> git web--browse --browser=custom someurl_with_&_in_it >actual &&\n> test_cmp expect actual\n>\n> That won't test that we are invoking kfmclient correctly, obviously, but\n> you can confirm at least that URLs are making it through to the browser\n> script intact.\n\nHmm, isn't '&' somewhat an unusual in URL? ...ah, not really, if it is in\nthe query parameter part it is quite common.\n\nThanks.\n"},{"id":"175793","messageId":"20110919182049.GA26115@sigill.intra.peff.net","threadId":"28411","inReplyTo":"7vvcso9zzi.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] git-web--browse: invoke kfmclient directly","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-09-19T18:20:49Z","receivedAt":"2011-09-19T18:20:49Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Sep 19, 2011 at 10:57:37AM -0700, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > Hmm. Actually, the one for custom browser commands might need it,\n> > because that one is expected to be a shell snippet. I suspect the\n> > simplest thing is to do something like:\n> >\n> > eval \"$browser_cmd \\\"\\$@\\\"\"\n> \n> Yeah, I agree, and the dq around $browser_cmd is kind of important, too,\n> for that to work and be readable.\n\nOops, good catch. Probably the most readable version would be:\n\n eval \"\\\"$browser_cmd\\\"\" '\"$@\"'\n\n-Peff\n"},{"id":"175794","messageId":"20110919183408.GB26115@sigill.intra.peff.net","threadId":"28411","inReplyTo":"1316424415-11156-1-git-send-email-judge.packham@gmail.com","subject":"Re: [RFC/PATCHv2] git-web--browse: avoid the use of eval","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-09-19T18:34:08Z","receivedAt":"2011-09-19T18:34:08Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Sep 19, 2011 at 09:26:55PM +1200, Chris Packham wrote:\n\n> Using eval causes problems when the URL contains an appropriately\n> escaped ampersand (\\&). Dropping eval from the built-in browser\n> invocation avoids the problem.\n> \n> Cc: peff@peff.net\n> Cc: chriscool@tuxfamily.org\n> Cc: jepler@unpythonic.net\n\nAlthough other projects do use \"cc\" in the commit message, I think we\ndon't usually bother adding this noise in the git project. The cc\nheaders in your email are enough.\n\n> I've replaced my tests With the test suggested by Peff (should I be\n> giving him credit in the copyright line or something?).\n\nFor a minor bit of help, usually mentioning the person in the commit\nmessage (with a \"Helped-by\", or indicating which parts they contributed\nto) is plenty. Personally, I don't even care much about that. My\ncontributions to git are thoroughly documented in the commit history and\nthe mailing list at this point. :)\n\nI also find the \"Copyright ...\" lines in the files to be overkill, too.\nThey end up becoming out-of-date as other people work on the file. The\ncommit history is the best way to get the right answer, and a comment in\nthe file is at best redundant with what's there. But that is just my\nopinion; I don't know that we have a particular policy for such\nthings[1].\n\n-Peff\n\n[1] Once upon a time, I think I saw the advice that every file should\nhave a copyright notice and mention the license at the top of the file,\nbut I don't know that it has ever been tested in court. I suppose the\ndistributed tarballs of a particular version would lack the copyright\nattribution, but in that case, my solution would be to generate it from\nthe commit history at packaging time.\n"},{"id":"175809","messageId":"7v62ko9scw.fsf@alter.siamese.dyndns.org","threadId":"28411","inReplyTo":"20110919182049.GA26115@sigill.intra.peff.net","subject":"Re: [PATCH] git-web--browse: invoke kfmclient directly","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-19T20:42:23Z","receivedAt":"2011-09-19T20:42:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Mon, Sep 19, 2011 at 10:57:37AM -0700, Junio C Hamano wrote:\n>\n>> Jeff King writes:\n>> \n>> > Hmm. Actually, the one for custom browser commands might need it,\n>> > because that one is expected to be a shell snippet. I suspect the\n>> > simplest thing is to do something like:\n>> >\n>> > eval \"$browser_cmd \\\"\\$@\\\"\"\n>> \n>> Yeah, I agree, and the dq around $browser_cmd is kind of important, too,\n>> for that to work and be readable.\n>\n> Oops, good catch. Probably the most readable version would be:\n>\n> eval \"\\\"$browser_cmd\\\"\" '\"$@\"'\n\nActually I didn't mean that double dq.\n\nIn fact, if browser_cmd is meant to be split as a shell snippet, I do not\nthink you want the string seen by eval to have dq around the expanded\nversion of $browser_cmd. And I tend to prefer feeding a single string to\neval, so the version in your message I quoted originally looks good to me.\n\nUnless I am missing something here...?\n"},{"id":"175810","messageId":"m2vcsotg7v.fsf@igel.home","threadId":"28411","inReplyTo":"20110919182049.GA26115@sigill.intra.peff.net","subject":"Re: [PATCH] git-web--browse: invoke kfmclient directly","fromName":"Andreas Schwab","fromEmail":"schwab@linux-m68k.org","sentAt":"2011-09-19T20:44:20Z","receivedAt":"2011-09-19T20:44:20Z","isPatch":true,"sender":{"key":"schwab@linux-m68k.org","avatar":"https://avatars.githubusercontent.com/u/2175493?v=4"},"body":"Jeff King writes:\n\n> On Mon, Sep 19, 2011 at 10:57:37AM -0700, Junio C Hamano wrote:\n>\n>> Jeff King writes:\n>> \n>> > Hmm. Actually, the one for custom browser commands might need it,\n>> > because that one is expected to be a shell snippet. I suspect the\n>> > simplest thing is to do something like:\n>> >\n>> > eval \"$browser_cmd \\\"\\$@\\\"\"\n>> \n>> Yeah, I agree, and the dq around $browser_cmd is kind of important, too,\n>> for that to work and be readable.\n>\n> Oops, good catch. Probably the most readable version would be:\n>\n> eval \"\\\"$browser_cmd\\\"\" '\"$@\"'\n\nThis make the use of eval even more questionable. If $browser_cmd is\nsupposed to be whitespace splitted then this won't do it (unless it\ncontains embedded double quotes).\n\nAndreas.\n\n-- \nAndreas Schwab, schwab@linux-m68k.org\nGPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5\n\"And now for something completely different.\"\n"},{"id":"175811","messageId":"20110919204448.GA3562@sigill.intra.peff.net","threadId":"28411","inReplyTo":"7v62ko9scw.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] git-web--browse: invoke kfmclient directly","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-09-19T20:44:48Z","receivedAt":"2011-09-19T20:44:48Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Sep 19, 2011 at 01:42:23PM -0700, Junio C Hamano wrote:\n\n> >> Yeah, I agree, and the dq around $browser_cmd is kind of important, too,\n> >> for that to work and be readable.\n> >\n> > Oops, good catch. Probably the most readable version would be:\n> >\n> > eval \"\\\"$browser_cmd\\\"\" '\"$@\"'\n> \n> Actually I didn't mean that double dq.\n> \n> In fact, if browser_cmd is meant to be split as a shell snippet, I do not\n> think you want the string seen by eval to have dq around the expanded\n> version of $browser_cmd. And I tend to prefer feeding a single string to\n> eval, so the version in your message I quoted originally looks good to me.\n> \n> Unless I am missing something here...?\n\nOh right. Sorry, I read your comment, thought that's what you meant, and\nthat I had overlooked something. Forgetting that it was intentional to\nleave off the quotes inside.\n\nSo yeah, my original is right. I just got turned around in all of the\ndiscussion.\n\nSorry for the noise.\n\n-Peff\n"},{"id":"175812","messageId":"7v1uvc9qhz.fsf@alter.siamese.dyndns.org","threadId":"28411","inReplyTo":"20110919204448.GA3562@sigill.intra.peff.net","subject":"Re: [PATCH] git-web--browse: invoke kfmclient directly","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-19T21:22:32Z","receivedAt":"2011-09-19T21:22:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Mon, Sep 19, 2011 at 01:42:23PM -0700, Junio C Hamano wrote:\n>\n>> >> Yeah, I agree, and the dq around $browser_cmd is kind of important, too,\n>> >> for that to work and be readable.\n>> >\n>> > Oops, good catch. Probably the most readable version would be:\n>> >\n>> > eval \"\\\"$browser_cmd\\\"\" '\"$@\"'\n>> \n>> Actually I didn't mean that double dq.\n>> \n>> In fact, if browser_cmd is meant to be split as a shell snippet, I do not\n>> think you want the string seen by eval to have dq around the expanded\n>> version of $browser_cmd. And I tend to prefer feeding a single string to\n>> eval, so the version in your message I quoted originally looks good to me.\n>> \n>> Unless I am missing something here...?\n>\n> Oh right. Sorry, I read your comment, thought that's what you meant, and\n> that I had overlooked something. Forgetting that it was intentional to\n> leave off the quotes inside.\n>\n> So yeah, my original is right. I just got turned around in all of the\n> discussion.\n\nThinking about it a bit more, I suspect that we should just let the 'eval'\ngrab value out of the $browser_cmd variable, i.e.\n\n\teval '$browser_cmd \"$@\"'\n\nno?\n"},{"id":"175814","messageId":"m38vpkrzhq.fsf@localhost.localdomain","threadId":"28411","inReplyTo":"7vvcso9zzi.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] git-web--browse: invoke kfmclient directly","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2011-09-19T21:32:24Z","receivedAt":"2011-09-19T21:32:24Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Junio C Hamano writes:\n> Jeff King writes:\n> \n> > If you want to make a test, I think you would do better with something\n> > like:\n> >\n> > echo someurl_with_&_in_it >expect &&\n> > git config browser.custom.cmd echo &&\n> > git web--browse --browser=custom someurl_with_&_in_it >actual &&\n> > test_cmp expect actual\n> >\n> > That won't test that we are invoking kfmclient correctly, obviously, but\n> > you can confirm at least that URLs are making it through to the browser\n> > script intact.\n> \n> Hmm, isn't '&' somewhat an unusual in URL? ...ah, not really, if it is in\n> the query parameter part it is quite common.\n\nIn newstyle URLs the name=value pairs in CGI parameter query string\nare separated with semicolons ';' rather than ampersands '&', because\nof problem with & <-> &.\n\nJust FYI.\n-- \nJakub Narębski\n"},{"id":"175816","messageId":"m2bougtdc2.fsf@igel.home","threadId":"28411","inReplyTo":"7v1uvc9qhz.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] git-web--browse: invoke kfmclient directly","fromName":"Andreas Schwab","fromEmail":"schwab@linux-m68k.org","sentAt":"2011-09-19T21:46:37Z","receivedAt":"2011-09-19T21:46:37Z","isPatch":true,"sender":{"key":"schwab@linux-m68k.org","avatar":"https://avatars.githubusercontent.com/u/2175493?v=4"},"body":"Junio C Hamano writes:\n\n> Thinking about it a bit more, I suspect that we should just let the 'eval'\n> grab value out of the $browser_cmd variable, i.e.\n>\n> \teval '$browser_cmd \"$@\"'\n>\n> no?\n\nThat's a Useless Use of Eval and 100% equivalent to this:\n\n$browser_cmd \"$@\"\n\nAndreas.\n\n-- \nAndreas Schwab, schwab@linux-m68k.org\nGPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5\n\"And now for something completely different.\"\n"},{"id":"175819","messageId":"20110919222325.GA4056@sigill.intra.peff.net","threadId":"28411","inReplyTo":"m2bougtdc2.fsf@igel.home","subject":"Re: [PATCH] git-web--browse: invoke kfmclient directly","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-09-19T22:23:25Z","receivedAt":"2011-09-19T22:23:25Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Sep 19, 2011 at 11:46:37PM +0200, Andreas Schwab wrote:\n\n> Junio C Hamano writes:\n> \n> > Thinking about it a bit more, I suspect that we should just let the 'eval'\n> > grab value out of the $browser_cmd variable, i.e.\n> >\n> > \teval '$browser_cmd \"$@\"'\n> >\n> > no?\n> \n> That's a Useless Use of Eval and 100% equivalent to this:\n> \n> $browser_cmd \"$@\"\n\nYeah. Doing:\n\n eval '$browser_cmd'\n\nwill do the whitespace-breaking we want, but it won't interpret actual\nshell magic characters, which we need in order to be compatible with\nother parts of git (which typically use \"sh -c ...\"). E.g.:\n\n foo=worked\n browser_cmd='echo $foo'\n # fail\n $browser_cmd\n # fail\n eval '$browser_cmd'\n # works\n eval \"$browser_cmd\"\n\n-Peff\n"},{"id":"175820","messageId":"7vk49488vi.fsf@alter.siamese.dyndns.org","threadId":"28411","inReplyTo":"20110919222325.GA4056@sigill.intra.peff.net","subject":"Re: [PATCH] git-web--browse: invoke kfmclient directly","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-19T22:28:33Z","receivedAt":"2011-09-19T22:28:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Yeah. Doing:\n>\n> eval '$browser_cmd'\n>\n> will do the whitespace-breaking we want, but it won't interpret actual\n> shell magic characters, which we need in order to be compatible with\n> other parts of git (which typically use \"sh -c ...\"). E.g.:\n>\n> foo=worked\n> browser_cmd='echo $foo'\n\nYikes, I forgot that we do that. You are right.\n"},{"id":"175847","messageId":"4E78572E.6030105@gmail.com","threadId":"28411","inReplyTo":"20110919183408.GB26115@sigill.intra.peff.net","subject":"Re: [RFC/PATCHv2] git-web--browse: avoid the use of eval","fromName":"Chris Packham","fromEmail":"judge.packham@gmail.com","sentAt":"2011-09-20T09:04:46Z","receivedAt":"2011-09-20T09:04:46Z","isPatch":false,"sender":{"key":"judge.packham@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155667?v=4"},"body":"On 20/09/11 06:34, Jeff King wrote:\n> On Mon, Sep 19, 2011 at 09:26:55PM +1200, Chris Packham wrote:\n> \n>> Using eval causes problems when the URL contains an appropriately\n>> escaped ampersand (\\&). Dropping eval from the built-in browser\n>> invocation avoids the problem.\n>>\n>> Cc: peff@peff.net\n>> Cc: chriscool@tuxfamily.org\n>> Cc: jepler@unpythonic.net\n> \n> Although other projects do use \"cc\" in the commit message, I think we\n> don't usually bother adding this noise in the git project. The cc\n> headers in your email are enough.\n\nThat's more for git send-email's benefit than anything else. I'm working\non a laptop with a touchpad (and a cat) so the less switching between\neditor and MUA the better. Any better suggestions for tracking Cc's for\ngit send-email?\n\n>> I've replaced my tests With the test suggested by Peff (should I be\n>> giving him credit in the copyright line or something?).\n> \n> For a minor bit of help, usually mentioning the person in the commit\n> message (with a \"Helped-by\", or indicating which parts they contributed\n> to) is plenty. Personally, I don't even care much about that. My\n> contributions to git are thoroughly documented in the commit history and\n> the mailing list at this point. :)\n> \n> I also find the \"Copyright ...\" lines in the files to be overkill, too.\n> They end up becoming out-of-date as other people work on the file. The\n> commit history is the best way to get the right answer, and a comment in\n> the file is at best redundant with what's there. But that is just my\n> opinion; I don't know that we have a particular policy for such\n> things[1].\n> \n> -Peff\n> \n> [1] Once upon a time, I think I saw the advice that every file should\n> have a copyright notice and mention the license at the top of the file,\n> but I don't know that it has ever been tested in court. I suppose the\n> distributed tarballs of a particular version would lack the copyright\n> attribution, but in that case, my solution would be to generate it from\n> the commit history at packaging time.\n\nThe example in t/README has has a copyright notice which is why I put\none in but I don't consider the test (or the fix itself) to actually be\ncopyrightable. If I wasn't creating a new file I wouldn't have bothered\nputting anything in (other than the testcase).\n"},{"id":"175878","messageId":"20110920184939.GA17322@sigill.intra.peff.net","threadId":"28411","inReplyTo":"4E78572E.6030105@gmail.com","subject":"Re: [RFC/PATCHv2] git-web--browse: avoid the use of eval","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-09-20T18:49:39Z","receivedAt":"2011-09-20T18:49:39Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Sep 20, 2011 at 09:04:46PM +1200, Chris Packham wrote:\n\n> > Although other projects do use \"cc\" in the commit message, I think we\n> > don't usually bother adding this noise in the git project. The cc\n> > headers in your email are enough.\n> \n> That's more for git send-email's benefit than anything else. I'm working\n> on a laptop with a touchpad (and a cat) so the less switching between\n> editor and MUA the better. Any better suggestions for tracking Cc's for\n> git send-email?\n\nIt would depend on your workflow, I think. You can use --cc to add\nheaders to format-patch. You could get very fancy and store them in\ngit-notes or somewhere else, and then pull them in with send-email's\ncc-cmd option. But I suspect you just want to stick them in the commit\nmessage one time and then have it used each time.\n\nIf put them after the double-dash line in your commit message, like:\n\n subject\n\n body\n ---\n cc: whoever\n\nThen that will be included verbatim in the mail by format-patch,\nsend-email will respect the cc line, and those lines will be dropped by\n\"git am\" when Junio applies the patch (they are still a slight noise to\nreaders of the mail, but at least they don't make it into the commit\nhistory).\n\n> The example in t/README has has a copyright notice which is why I put\n> one in but I don't consider the test (or the fix itself) to actually be\n> copyrightable. If I wasn't creating a new file I wouldn't have bothered\n> putting anything in (other than the testcase).\n\nYeah, that's why I said I don't know if we have a policy. We clearly\nhave a lot of copyright statements, but they are all horribly out of\ndate. I was hoping Junio might weigh in.\n\n-Peff\n"},{"id":"175883","messageId":"7vvcsn57na.fsf@alter.siamese.dyndns.org","threadId":"28411","inReplyTo":"20110920184939.GA17322@sigill.intra.peff.net","subject":"Re: [RFC/PATCHv2] git-web--browse: avoid the use of eval","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-20T19:35:37Z","receivedAt":"2011-09-20T19:35:37Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n>> The example in t/README has has a copyright notice which is why I put\n>> one in but I don't consider the test (or the fix itself) to actually be\n>> copyrightable. If I wasn't creating a new file I wouldn't have bothered\n>> putting anything in (other than the testcase).\n>\n> Yeah, that's why I said I don't know if we have a policy. We clearly\n> have a lot of copyright statements, but they are all horribly out of\n> date. I was hoping Junio might weigh in.\n\nTo be honest, I do not care very much either way. From the licensing point\nof view we know everything is covered by the top-level COPYING unless\notherwise noted explicitly in an individual file (which is not the case\nfor this patch anyway), and even without the copyright notice we can trace\nwhere the files come from with \"git log\", so these three lines in a small\ntest file are essentially noise, not very useful but are not irritating\nenough to warrant an effort from me to amend it out.\n\nI do have a mild objection to a patch that adds a new copyright notice\nline to an existing file when it adds only a few new lines, though. When\nthe code is refactored and these new lines are made unneeded, it is likely\nthat nobody would bother removing that copyright notice line that names\nthe author of the patch that added these lines.\n"},{"id":"175973","messageId":"20110922013101.GB26880@unpythonic.net","threadId":"28411","inReplyTo":"m3hb49sn26.fsf@localhost.localdomain","subject":"Re: [RFC/PATCH] Configurable hyperlinking in gitk","fromName":"Jeff Epler","fromEmail":"jepler@unpythonic.net","sentAt":"2011-09-22T01:31:01Z","receivedAt":"2011-09-22T01:31:01Z","isPatch":true,"sender":{"key":"jepler@unpythonic.net","avatar":"https://avatars.githubusercontent.com/u/1517291?v=4"},"body":"On Sun, Sep 18, 2011 at 11:50:30AM -0700, Jakub Narebski wrote:\n> Perhaps more descriptive name, i.e.\n> \n> linkify..regexp\n> linkify..subst\n> \n> would be better?\n> \n> I guess that regexp is an extended regular expression, isn't it?\n\nIf \"regexp\" is clearer than \"re\" then I have no quarrel with changing\nit. The typical user won't be typing these over and over, so the value\nof brevity is limited.\n\nAs written, it's whatever is accepted by tcl's regular expression\nmatcher, which is described in re_syntax(n), installed as\nre_syntax(3tcl) on debian-derived systems. A one-sentence summary of a\nTCL \"ARE\" is \"basically EREs with some significant extensions\".\n\nIt is probably possible to write expressions that are going to work the\nsame in tcl, perl, and posix regular expressions, but to some extent the\nuser who writes a complex expression and then tries to use it with both\ngitk and a future gitweb will simply be permitted to keep both pieces\nwhen it breaks.\n\nIs it unnecessarily complicated to design\n linkify..(regexp|subst)\n*AND*\n gitk.linkify..(regexp|subst)\nin from the start? This way the hypothetical power user can write a\ndifferent version of the expression for gitk and future gitweb if it is\nrequired by RE dialect differences.\n\nJeff\n"},{"id":"175977","messageId":"20110922021526.GC26880@unpythonic.net","threadId":"28411","inReplyTo":"20110922013101.GB26880@unpythonic.net","subject":"[PATCH v3] Configurable hyperlinking in gitk","fromName":"Jeff Epler","fromEmail":"jepler@unpythonic.net","sentAt":"2011-09-22T02:15:26Z","receivedAt":"2011-09-22T02:15:26Z","isPatch":true,"sender":{"key":"jepler@unpythonic.net","avatar":"https://avatars.githubusercontent.com/u/1517291?v=4"},"body":"Many projects use project-specific notations in changelogs to refer\nto bug trackers and the like. One example is the \"Closes: #12345\"\nnotation used in Debian.\n\nMake gitk configurable so that arbitrary strings can be turned into\nclickable links that are opened in a web browser.\n\nSigned-off-by: Jeff Epler \n---\nSince the previous patch, I\n * Renamed configuration variables to get rid of the \"gitk\" prefix\n to encourage other git-related programs to adopt the same\n functionality.\n\n * Renamed configuration variables from cryptic \".re\", \".sub\" to less\n cryptic \".regexp\" and \"subst\"\n\n * Changed the example RE to be an ERE (no \\d or \\M)\n\n * Documented that these are POSIX EREs; hopefully that's OK. I see\n in CodingGuidelines that in git itself \"a subset of BREs\" are used,\n so maybe even this is too much power. And hopefully tcl's\n re_syntax really is close enough to an ERE superset that this isn't\n a terrible lie about the initial implementation either.\n\n * Added a Signed-Off-By, since I've had a number of positive feedbacks\n and the only problems I've heard of (since patch v2) are the ones\n related to 'eval' in git-web--browse.\n\nIn v2 of the patch, I had fixed a problem with %-signs in URLs and\nchanged the documentation example.\n\n Documentation/config.txt | 30 +++++++++++++++++-\n gitk-git/gitk | 75 +++++++++++++++++++++++++++++++++++++++++++++-\n 2 files changed, 102 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex ae9913b..ffc9ccf 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1064,6 +1064,10 @@ All gitcvs variables except for 'gitcvs.usecrlfattr' and\n is one of \"ext\" and \"pserver\") to make them apply only for the given\n access method.\n \n+gitk.browser::\n+\tSpecify the browser that will be used to open links generated by\n+\t'linkify' configuration options.\n+\n grep.lineNumber::\n \tIf set to true, enable '-n' option by default.\n \n@@ -1317,6 +1321,28 @@ interactive.singlekey::\n \tsetting is silently ignored if portable keystroke input\n \tis not available.\n \n+linkify..regexp::\n+\tSpecify a regular expression in the POSIX Extended Regular Expression\n+\tsyntax defining a class of strings to automatically convert to\n+\thyperlinks. This regular expression many not span multiple lines.\n+\tYou must also specify 'linkify..subst'.\n+\n+linkify..subst::\n+\tSpecify a substitution that results in the target URL for the\n+\trelated regular expression. Back-references like '\\1' refer\n+\tto capturing groups in the associated regular expression.\n+\tYou must also specify 'linkify..regexp'.\n++\n+For example, to automatically link from Debian-style \"Closes: #nnnn\"\n+message to the Debian BTS,\n++\n+--------\n+ git config linkify.debian-bts.regexp '#([1-9][0-9]*)'\n+ git config linkify.debian-bts.subst 'http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=\\1'\n+--------\n++\n+Currently, only linkgit:gitk[1] converts strings to links in this fashion.\n+\n log.abbrevCommit::\n \tIf true, makes linkgit:git-log[1], linkgit:git-show[1], and\n \tlinkgit:git-whatchanged[1] assume `\\--abbrev-commit`. You may\n@@ -1870,5 +1896,5 @@ user.signingkey::\n \n web.browser::\n \tSpecify a web browser that may be used by some commands.\n-\tCurrently only linkgit:git-instaweb[1] and linkgit:git-help[1]\n-\tmay use it.\n+\tCurrently only linkgit:git-instaweb[1], linkgit:gitk[1],\n+\tand linkgit:git-help[1] may use it.\ndiff --git a/gitk-git/gitk b/gitk-git/gitk\nindex 4cde0c4..9db5525 100755\n--- a/gitk-git/gitk\n+++ b/gitk-git/gitk\n@@ -6684,7 +6684,7 @@ proc commit_descriptor {p} {\n # append some text to the ctext widget, and make any SHA1 ID\n # that we know about be a clickable link.\n proc appendwithlinks {text tags} {\n- global ctext linknum curview\n+ global ctext linknum curview linkmakers\n \n set start [$ctext index \"end - 1c\"]\n $ctext insert end $text $tags\n@@ -6699,6 +6699,30 @@ proc appendwithlinks {text tags} {\n \tsetlink $linkid link$linknum\n \tincr linknum\n }\n+\n+ if {$linkmakers == {}} return\n+\n+ set link_re {}\n+ foreach {re rep} $linkmakers { lappend link_re $re }\n+ set link_re \"([join $link_re {)|(}])\"\n+\n+ set ee 0\n+ while {[regexp -indices -start $ee -- $link_re $text l]} {\n+\tset s [lindex $l 0]\n+\tset e [lindex $l 1]\n+\tset linktext [string range $text $s $e]\n+\tincr e\n+\tset ee $e\n+\n+\tforeach {re rep} $linkmakers {\n+\t if {![regsub $re $linktext $rep linkurl]} continue\n+\t $ctext tag delete link$linknum\n+\t $ctext tag add link$linknum \"$start + $s c\" \"$start + $e c\"\n+\t seturllink $linkurl link$linknum\n+\t incr linknum\n+\t break\n+\t}\n+ }\n }\n \n proc setlink {id lk} {\n@@ -6726,6 +6750,53 @@ proc setlink {id lk} {\n }\n }\n \n+proc get_link_config {} {\n+ if {[catch {exec git config -z --get-regexp {^linkify\\.}} linkers]} {\n+\treturn {}\n+ }\n+\n+ set linktypes [list]\n+ foreach item [split $linkers \"\\0\"] {\n+\tif {$item == \"\"} continue\n+\tif {![regexp {linkify\\.(\\S+)\\.(regexp|subst)\\s(.*)} $item _ k t v]} {\n+\t continue\n+\t}\n+\tset linkconfig($t,$k) $v\n+\tif {$t == \"regexp\"} { lappend linktypes $k }\n+ }\n+\n+ set linkmakers [list]\n+ foreach k $linktypes {\n+\tif {![info exists linkconfig(subst,$k)]} {\n+\t puts stderr \"Warning: link `$k' is missing a substitution string\"\n+\t} elseif {[catch {regexp -inline -- $linkconfig(regexp,$k) \"\"} err]} {\n+\t puts stderr \"Warning: link `$k': $err\"\n+\t} else {\n+\t lappend linkmakers $linkconfig(regexp,$k) $linkconfig(subst,$k)\n+\t}\n+\tunset linkconfig(regexp,$k)\n+\tunset -nocomplain linkconfig(subst,$k)\n+ }\n+ foreach k [array names linkconfig] {\n+\tregexp \"subst,(.*)\" $k _ k\n+\tputs stderr \"Warning: link `$k' is missing a regular expression\"\n+ }\n+ set linkmakers\n+}\n+\n+proc openlink {url} {\n+ exec git web--browse --config=gitk.browser $url &\n+}\n+\n+proc seturllink {url lk} {\n+ set qurl [string map {% %%} $url]\n+ global ctext\n+ $ctext tag conf $lk -foreground blue -underline 1\n+ $ctext tag bind $lk <1> [list openlink $qurl]\n+ $ctext tag bind $lk {linkcursor %W 1}\n+ $ctext tag bind $lk {linkcursor %W -1}\n+}\n+\n proc appendshortlink {id {pre {}} {post {}}} {\n global ctext linknum\n \n@@ -11693,6 +11764,8 @@ if {[tk windowingsystem] eq \"win32\"} {\n focus -force .\n }\n \n+set linkmakers [get_link_config]\n+\n getcommits {}\n \n # Local variables:\n-- \n1.7.2.5\n"},{"id":"177390","messageId":"20111011183722.GA26646@unpythonic.net","threadId":"28411","inReplyTo":"20110922013101.GB26880@unpythonic.net","subject":"[RESEND PATCH v3] Configurable hyperlinking in gitk","fromName":"Jeff Epler","fromEmail":"jepler@unpythonic.net","sentAt":"2011-10-11T18:37:23Z","receivedAt":"2011-10-11T18:37:23Z","isPatch":true,"sender":{"key":"jepler@unpythonic.net","avatar":"https://avatars.githubusercontent.com/u/1517291?v=4"},"body":"Many projects use project-specific notations in changelogs to refer\nto bug trackers and the like. One example is the \"Closes: #12345\"\nnotation used in Debian.\n\nMake gitk configurable so that arbitrary strings can be turned into\nclickable links that are opened in a web browser.\n\nSigned-off-by: Jeff Epler \n---\nThis v3 patch didn't generate any discussion last time around (~3 weeks\nago), so I've taken the liberty of reposting it.\n\nI'm aware of no problems with this patch, and a number of people have\ncommented that it is useful to them. For URLs that contain \"&\" and\nother shell metacharacters, it *does* depend on r480f062c\n\"git-web--browse: avoid the use of eval\" which is in next but not maint.\n\nSince the V2 patch, I\n * Renamed configuration variables to get rid of the \"gitk\" prefix\n to encourage other git-related programs to adopt the same\n functionality.\n\n * Renamed configuration variables from cryptic \"re\", \"sub\" to less\n cryptic \"regexp\" and \"subst\"\n\n * Changed the example RE to be an ERE (no \\d or \\M)\n\n * Documented that these are POSIX EREs; hopefully that's OK. I see\n in CodingGuidelines that in git itself \"a subset of BREs\" are used,\n so maybe even this is too much power. And hopefully tcl's\n re_syntax really is close enough to an ERE superset that this isn't\n a terrible lie about the initial implementation either.\n\n * Added a Signed-Off-By, since I've had a number of positive feedbacks\n and the only problems I've heard of (since patch v2) are the ones\n related to 'eval' in git-web--browse.\n\nIn v2 of the patch, I had fixed a problem with %-signs in URLs and\nchanged the documentation example.\n\n Documentation/config.txt | 30 +++++++++++++++++-\n gitk-git/gitk | 75 +++++++++++++++++++++++++++++++++++++++++++++-\n 2 files changed, 102 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex ae9913b..ffc9ccf 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1064,6 +1064,10 @@ All gitcvs variables except for 'gitcvs.usecrlfattr' and\n is one of \"ext\" and \"pserver\") to make them apply only for the given\n access method.\n \n+gitk.browser::\n+\tSpecify the browser that will be used to open links generated by\n+\t'linkify' configuration options.\n+\n grep.lineNumber::\n \tIf set to true, enable '-n' option by default.\n \n@@ -1317,6 +1321,28 @@ interactive.singlekey::\n \tsetting is silently ignored if portable keystroke input\n \tis not available.\n \n+linkify..regexp::\n+\tSpecify a regular expression in the POSIX Extended Regular Expression\n+\tsyntax defining a class of strings to automatically convert to\n+\thyperlinks. This regular expression many not span multiple lines.\n+\tYou must also specify 'linkify..subst'.\n+\n+linkify..subst::\n+\tSpecify a substitution that results in the target URL for the\n+\trelated regular expression. Back-references like '\\1' refer\n+\tto capturing groups in the associated regular expression.\n+\tYou must also specify 'linkify..regexp'.\n++\n+For example, to automatically link from Debian-style \"Closes: #nnnn\"\n+message to the Debian BTS,\n++\n+--------\n+ git config linkify.debian-bts.regexp '#([1-9][0-9]*)'\n+ git config linkify.debian-bts.subst 'http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=\\1'\n+--------\n++\n+Currently, only linkgit:gitk[1] converts strings to links in this fashion.\n+\n log.abbrevCommit::\n \tIf true, makes linkgit:git-log[1], linkgit:git-show[1], and\n \tlinkgit:git-whatchanged[1] assume `\\--abbrev-commit`. You may\n@@ -1870,5 +1896,5 @@ user.signingkey::\n \n web.browser::\n \tSpecify a web browser that may be used by some commands.\n-\tCurrently only linkgit:git-instaweb[1] and linkgit:git-help[1]\n-\tmay use it.\n+\tCurrently only linkgit:git-instaweb[1], linkgit:gitk[1],\n+\tand linkgit:git-help[1] may use it.\ndiff --git a/gitk-git/gitk b/gitk-git/gitk\nindex 4cde0c4..9db5525 100755\n--- a/gitk-git/gitk\n+++ b/gitk-git/gitk\n@@ -6684,7 +6684,7 @@ proc commit_descriptor {p} {\n # append some text to the ctext widget, and make any SHA1 ID\n # that we know about be a clickable link.\n proc appendwithlinks {text tags} {\n- global ctext linknum curview\n+ global ctext linknum curview linkmakers\n \n set start [$ctext index \"end - 1c\"]\n $ctext insert end $text $tags\n@@ -6699,6 +6699,30 @@ proc appendwithlinks {text tags} {\n \tsetlink $linkid link$linknum\n \tincr linknum\n }\n+\n+ if {$linkmakers == {}} return\n+\n+ set link_re {}\n+ foreach {re rep} $linkmakers { lappend link_re $re }\n+ set link_re \"([join $link_re {)|(}])\"\n+\n+ set ee 0\n+ while {[regexp -indices -start $ee -- $link_re $text l]} {\n+\tset s [lindex $l 0]\n+\tset e [lindex $l 1]\n+\tset linktext [string range $text $s $e]\n+\tincr e\n+\tset ee $e\n+\n+\tforeach {re rep} $linkmakers {\n+\t if {![regsub $re $linktext $rep linkurl]} continue\n+\t $ctext tag delete link$linknum\n+\t $ctext tag add link$linknum \"$start + $s c\" \"$start + $e c\"\n+\t seturllink $linkurl link$linknum\n+\t incr linknum\n+\t break\n+\t}\n+ }\n }\n \n proc setlink {id lk} {\n@@ -6726,6 +6750,53 @@ proc setlink {id lk} {\n }\n }\n \n+proc get_link_config {} {\n+ if {[catch {exec git config -z --get-regexp {^linkify\\.}} linkers]} {\n+\treturn {}\n+ }\n+\n+ set linktypes [list]\n+ foreach item [split $linkers \"\\0\"] {\n+\tif {$item == \"\"} continue\n+\tif {![regexp {linkify\\.(\\S+)\\.(regexp|subst)\\s(.*)} $item _ k t v]} {\n+\t continue\n+\t}\n+\tset linkconfig($t,$k) $v\n+\tif {$t == \"regexp\"} { lappend linktypes $k }\n+ }\n+\n+ set linkmakers [list]\n+ foreach k $linktypes {\n+\tif {![info exists linkconfig(subst,$k)]} {\n+\t puts stderr \"Warning: link `$k' is missing a substitution string\"\n+\t} elseif {[catch {regexp -inline -- $linkconfig(regexp,$k) \"\"} err]} {\n+\t puts stderr \"Warning: link `$k': $err\"\n+\t} else {\n+\t lappend linkmakers $linkconfig(regexp,$k) $linkconfig(subst,$k)\n+\t}\n+\tunset linkconfig(regexp,$k)\n+\tunset -nocomplain linkconfig(subst,$k)\n+ }\n+ foreach k [array names linkconfig] {\n+\tregexp \"subst,(.*)\" $k _ k\n+\tputs stderr \"Warning: link `$k' is missing a regular expression\"\n+ }\n+ set linkmakers\n+}\n+\n+proc openlink {url} {\n+ exec git web--browse --config=gitk.browser $url &\n+}\n+\n+proc seturllink {url lk} {\n+ set qurl [string map {% %%} $url]\n+ global ctext\n+ $ctext tag conf $lk -foreground blue -underline 1\n+ $ctext tag bind $lk <1> [list openlink $qurl]\n+ $ctext tag bind $lk {linkcursor %W 1}\n+ $ctext tag bind $lk {linkcursor %W -1}\n+}\n+\n proc appendshortlink {id {pre {}} {post {}}} {\n global ctext linknum\n \n@@ -11693,6 +11764,8 @@ if {[tk windowingsystem] eq \"win32\"} {\n focus -force .\n }\n \n+set linkmakers [get_link_config]\n+\n getcommits {}\n \n # Local variables:\n-- \n1.7.2.5\n"},{"id":"177403","messageId":"7vfwizdvnn.fsf@alter.siamese.dyndns.org","threadId":"28411","inReplyTo":"20111011183722.GA26646@unpythonic.net","subject":"Re: [RESEND PATCH v3] Configurable hyperlinking in gitk","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-11T22:13:16Z","receivedAt":"2011-10-11T22:13:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff Epler writes:\n\n> I'm aware of no problems with this patch, and a number of people have\n> commented that it is useful to them.\n\nHmmm, \"didn't generate any discussion\" does not mesh very well with \"a\nnumber of people are happy\". Which one should I trust?\n\n> * Documented that these are POSIX EREs; hopefully that's OK. I see\n> in CodingGuidelines that in git itself \"a subset of BREs\" are used,\n> so maybe even this is too much power. And hopefully tcl's\n> re_syntax really is close enough to an ERE superset that this isn't\n> a terrible lie about the initial implementation either.\n\nI think it is better to be honest and say these are fed to the native\nregexp engine of Tcl somewhere in the documentation.\n\nDeclaring \"these are POSIX EREs\" invites a user to expect they truly\nare. When the pattern the user wrote triggers a strange Tcl extension to\ncause unexpected things to match, the documentation needs to help the user\nto understand why. I understand the longer-term wish to reuse these in\ngitweb and elsewhere, but it becomes even more important that it is\nclearly documented that these \"regexp\" are fed to native regexp engines of\nTcl and Perl depending on the program that they are used in. Unless the\ndocumentation spells it out, the user will not be able to decide how to\nwork the implementation around, avoiding constructs that would behave\ndifferently between Tcl and Perl.\n\nDoesn't tcl have/use pcre these days, by the way? If we envision that this\nwill be shared with gitweb, perhaps using that might be a better option to\nreduce potential confusion.\n\n> * Added a Signed-Off-By, since I've had a number of positive feedbacks\n> and the only problems I've heard of (since patch v2) are the ones\n> related to 'eval' in git-web--browse.\n\nBy the way, \"This patch is good\" does not have anything to do with signing\noff a patch.\n\nPaul wanted to keep gitk sources separately available from the rest of the\ngit. After all, that is how gitk project started. Even after 5569bf9 (Do a\ncross-project merge of Paul Mackerras' gitk visualizer, 2005-06-22), we\nkept it so that git://git.kernel.org/pub/scm/gitk/gitk was the primary\nproject to make changes to gitk, and git.git pulled from it (it is an\nassymmetric pull, as gitk cannot pull from git without contaminating its\nhistory with the changes to the rest of git).\n\nI do not know how motivated Paul is to keep gitk part separated in its own\nproject these days. I do not think the /pub/scm/gitk/gitk repository has\nbeen re-populated yet. Assuming that it will eventually come back on-line,\ncould you send the gitk part of this change to Paul (i.e. the diff header\nof your patch should read \"diff --git a/gitk b/gitk\") and another separate\npatch to Documentation/ part?\n\nPaul, if you are orphaning gitk, I do _not_ mind start taking patches that\ntouch gitk myself directly into git tree.\n\nBut I would still need reviewers who are motivated and interested in\nenhancing and maintaining gitk.\n\n> +linkify..regexp::\n> +\tSpecify a regular expression in the POSIX Extended Regular Expression\n> +\tsyntax defining a class of strings to automatically convert to\n> +\thyperlinks. This regular expression many not span multiple lines.\n> +\tYou must also specify 'linkify..subst'.\n\nSaying \"You must ...\" without explicitly saying \"why\" is a bad style. If\nthe reader already _knows_ the .regexp is used to supply captured\nsubstring to the corresponding .subst, then it is obvious that whenever\nyou have .regexp you need a matching .subst, but that is not even\nexplained here.\n\nHow about this?\n\n\tA string that matches this regexp is converted to a hyperlink\n\tusing the value of corresponding `linkify..subst` variable.\n\tThe regular expression is passed to the regexp engine of Tcl (in\n\tgitk) or Perl (in gitweb).\n\n> +linkify..subst::\n> +\tSpecify a substitution that results in the target URL for the\n> +\trelated regular expression. Back-references like '\\1' refer\n> +\tto capturing groups in the associated regular expression.\n> +\tYou must also specify 'linkify..regexp'.\n\nLikewise.\n\n\tA string matched the value of the corresponding\n\t`linkify..regexp` variable is rewritten to this URL. The\n\tvalue of this variable can contain back-references like `\\1` to\n\trefer to capturing groups in the associated regular expression.\n"},{"id":"177433","messageId":"4E9558D0.60802@gmail.com","threadId":"28411","inReplyTo":"7vfwizdvnn.fsf@alter.siamese.dyndns.org","subject":"Re: [RESEND PATCH v3] Configurable hyperlinking in gitk","fromName":"Chris Packham","fromEmail":"judge.packham@gmail.com","sentAt":"2011-10-12T09:07:28Z","receivedAt":"2011-10-12T09:07:28Z","isPatch":true,"sender":{"key":"judge.packham@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155667?v=4"},"body":"On 12/10/11 11:13, Junio C Hamano wrote:\n> Jeff Epler writes:\n> \n>> I'm aware of no problems with this patch, and a number of people have\n>> commented that it is useful to them.\n> \n> Hmmm, \"didn't generate any discussion\" does not mesh very well with \"a\n> number of people are happy\". Which one should I trust?\n> \n\nFor what it's worth I've (just) tested v3 and it works well for me.\n"}]}