Re: [RESEND PATCH v3] Configurable hyperlinking in gitk

Jeff Epler <jepler@unpythonic.net> writes:

Hmmm, "didn't generate any discussion" does not mesh very well with "a number of people are happy". Which one should I trust?

I think it is better to be honest and say these are fed to the native regexp engine of Tcl somewhere in the documentation.

Declaring "these are POSIX EREs" invites a user to expect they truly are. When the pattern the user wrote triggers a strange Tcl extension to cause unexpected things to match, the documentation needs to help the user to understand why. I understand the longer-term wish to reuse these in gitweb and elsewhere, but it becomes even more important that it is clearly documented that these "regexp" are fed to native regexp engines of Tcl and Perl depending on the program that they are used in. Unless the documentation spells it out, the user will not be able to decide how to work the implementation around, avoiding constructs that would behave differently between Tcl and Perl.

Doesn't tcl have/use pcre these days, by the way? If we envision that this will be shared with gitweb, perhaps using that might be a better option to reduce potential confusion.

By the way, "This patch is good" does not have anything to do with signing off a patch.

Paul wanted to keep gitk sources separately available from the rest of the git. After all, that is how gitk project started. Even after 5569bf9 (Do a cross-project merge of Paul Mackerras' gitk visualizer, 2005-06-22), we kept it so that git://git.kernel.org/pub/scm/gitk/gitk was the primary project to make changes to gitk, and git.git pulled from it (it is an assymmetric pull, as gitk cannot pull from git without contaminating its history with the changes to the rest of git).

I do not know how motivated Paul is to keep gitk part separated in its own project these days. I do not think the /pub/scm/gitk/gitk repository has been re-populated yet. Assuming that it will eventually come back on-line, could you send the gitk part of this change to Paul (i.e. the diff header of your patch should read "diff --git a/gitk b/gitk") and another separate patch to Documentation/ part?

Paul, if you are orphaning gitk, I do _not_ mind start taking patches that touch gitk myself directly into git tree.

But I would still need reviewers who are motivated and interested in enhancing and maintaining gitk.

Saying "You must ..." without explicitly saying "why" is a bad style. If the reader already _knows_ the .regexp is used to supply captured substring to the corresponding .subst, then it is obvious that whenever you have .regexp you need a matching .subst, but that is not even explained here.

How about this?

	A string that matches this regexp is converted to a hyperlink
	using the value of corresponding `linkify.<name>.subst` variable.
	The regular expression is passed to the regexp engine of Tcl (in
	gitk) or Perl (in gitweb).

Likewise.

	A string matched the value of the corresponding
	`linkify.<name>.regexp` variable is rewritten to this URL. The
	value of this variable can contain back-references like `\1` to
	refer to capturing groups in the associated regular expression.