Skip installing .revision-hash if not running make install from a git checkout

[phy1729]

Check --show-prefix rather than --is-inside-work-tree in case we're
inside some other git repository (e.g. a ports tree).

[danielshahaf]

+1 on the concept, but a question, see below.

[danielshahaf]

The incumbent code always creates .revision-hash; the PR does not. Should it?

This would manifest when running make install in a release tarball, for example.

[phy1729]

I don't think so. The only place that .revision-hash is used is when defining ZSH_HIGHLIGHT_REVISION modified else-patch. There are three cases.

1. Running from a git checkout. (ZSH_HIGHLIGHT_REVISION=HEAD)
2. Running from a make install'd from a git checkout copy (ZSH_HIGHLIGHT_REVISION=revision_id)
3. Running from a make install'd from a non-git checkout copy (with PR ZSH_HIGHLIGHT_REVISION is not set; previously HEAD)

ZSH_HIGHLIGHT_REVISION=HEAD doesn't give any extra information; however, it's an extra file to install and does cost a bit of time to do the pattern match in zsh-syntax-highlighting.zsh. Further since the third case should only happen from a release tarball, ZSH_HIGHLIGHT_VERSION should be sufficient to know which version of the code is in use.

[danielshahaf]

Running from a make install'd from a non-git checkout copy (with PR ZSH_HIGHLIGHT_REVISION is not set; previously HEAD)

Factually wrong:

$ zsh -f 
% (cd ~/src/z-sy-h && git archive --format=tar 0.7.1) | tar xf -  
% make install PREFIX=$PWD/d
% source d/share/zsh-syntax-highlighting/zsh-syntax-highlighting.zsh 
% typeset -pm \*SION               
typeset ZSH_HIGHLIGHT_VERSION=0.7.1
typeset ZSH_HIGHLIGHT_REVISION=932e29a0c75411cb618f02995b66c0a4a25699bc
% 

ZSH_HIGHLIGHT_REVISION=HEAD doesn't give any extra information; however, it's an extra file to install and does cost a bit of time to do the pattern match in zsh-syntax-highlighting.zsh.

Wait, are you saying that the test [[ $ZSH_HIGHLIGHT_REVISION == \$Format:* ]] accounts for a measurable fraction of the start-up time? That'd be quite surprising. Perhaps it's the glob operator that slows it down? Is either [[ ${ZSH_HIGHLIGHT_REVISION[1,8]} == \$Format: ]] or [[ ${ZSH_HIGHLIGHT_REVISION[1,8]} =~ '\$Format:' ]] faster the incumbent code?

We can't just use that substring trick without further ado because it breaks under KSH_ARRAYS. (That's another form of #688. I have a pending fix for that issue, but that fix should be merged after redrawhook in order to not confuse git merge.)

Further since the third case should only happen from a release tarball, ZSH_HIGHLIGHT_VERSION should be sufficient to know which version of the code is in use.

I see your point, but I'm not sold. Right now, there's an invariant that git show $ZSH_HIGHLIGHT_REVISION will always DTRT regardless of whether one is in case 1/2/3. That's also useful when doing support: I can ask someone to echo $ZSH_HIGHLIGHT_REVISION and it will always DTRT regardless of how they got z-sy-h. One, short command. Making that variable undefined in one case would break these.

Circling back to your original issue $ZSH_HIGHLIGHT_REVISION having a wrong value after make install from within another git repository, how about another approach:

diff --git a/.gitattributes b/.gitattributes
index 715e624..65b8d75 100644
--- a/.gitattributes
+++ b/.gitattributes
@@ -1 +1,2 @@
+# If you change the contents of .revision-hash, update the 'install' target in Makefile.
 .revision-hash export-subst
diff --git a/INSTALL.md b/INSTALL.md
index 23acdf5..70bc538 100644
--- a/INSTALL.md
+++ b/INSTALL.md
@@ -125,3 +125,13 @@ source /usr/local/share/zsh-syntax-highlighting/zsh-syntax-highlighting.zsh
 ```
 
 to their `.zshrc`s.
+
+### For packagers
+
+Packagers are asked to ensure that the values reported by `typeset -p
+ZSH_HIGHLIGHT_VERSION ZSH_HIGHLIGHT_REVISION` in the installed packages
+are accurate.  In particular, packagers who apply patches to the upstream
+releases are asked to munge the values of those two variables accordingly.
+(For example, Debian sets the values to `0.7.1-1_debian` and `debian/0.7.1-1`
+respectively, the latter being the name of a tag in the downstream package's
+git repository.)
diff --git a/Makefile b/Makefile
index bbc1d43..83cbeba 100644
--- a/Makefile
+++ b/Makefile
@@ -18,8 +18,14 @@ install: all
 	cp .version zsh-syntax-highlighting.zsh $(SHARE_DIR)
 	cp COPYING.md README.md changelog.md $(DOC_DIR)
 	sed -e '1s/ .*//' -e '/^\[build-status-[a-z]*\]: /d' < README.md > $(DOC_DIR)/README.md
-	if [ x"true" = x"`git rev-parse --is-inside-work-tree 2>/dev/null`" ]; then \
-		git rev-parse HEAD; \
+# Fabricate $(SHARE_DIR)/.revision-hash, if needed.  The contents of ./.revision-hash depend
+# on whether we're running from git or from a release tarball.
+	if grep -q '[$$]Format:.*[$$]' <.revision-hash; then \
+		if prefix=`git rev-parse --show-prefix 2>/dev/null` && [ x"$$prefix" = x ]; then \
+			git rev-parse HEAD; \
+		else \
+			echo "(unknown; report this as a bug to your distro packager)" \
+		fi \
 	else \
 		cat .revision-hash; \
 	fi > $(SHARE_DIR)/.revision-hash
diff --git a/changelog.md b/changelog.md
index 496b4a8..76c9b86 100644
--- a/changelog.md
+++ b/changelog.md
@@ -73,6 +73,9 @@
 
 - Recognize `env` as a precommand (e.g., `env FOO=bar ls`)
 
+- Correct the value of `$ZSH_HIGHLIGHT_REVISION` in an edge case
+  [#723]
+
 # Changes in version 0.7.1
 
 - Remove out-of-date information from the 0.7.0 changelog.

It's based on your fix, but I edited the changelog entry to use $ZSH_HIGHLIGHT_REVISION, which is the public API term.

That leaves the pattern match in the driver, though. I'd be surprised if it's actually a performance hot spot, but if it is, let's handle it.

Hope this doesn't come across as bikeshedding — that's not my intention.