Fix zcompile error with zsh 5.4

[ericbn]

Error is:

$ zcompile highlighters/main/test-data/function.zsh
zsh: defining function based on alias `ls'
zsh: parse error near `()'
zcompile: can't read file: highlighters/main/test-data/function.zsh

View Incompatibilities since 5.3.1 in https://github.com/zsh-users/zsh/blob/bb218704d27bcca9aa4426296dcd5c13d58b330a/README

[danielshahaf]

I'm aware of the upstream change, thanks, see 9523d6d.

What I don't understand is why this is needed in the first place. z-sy-h does not alias ls anywhere, and in fact, it goes into some trouble to ensure that it won't be affected by any alias ls=... that .zshrc might be doing:

zsh-syntax-highlighting/zsh-syntax-highlighting.zsh

Lines 30 to 36 in 4cde79e

	# First of all, ensure predictable parsing.
	zsh_highlight__aliases=`builtin alias -Lm '[^+]*'`
	# In zsh <= 5.2, `alias -L` emits aliases that begin with a plus sign ('alias -- +foo=42')
	# them without a '--' guard, so they don't round trip.
	#
	# Hence, we exclude them from unaliasing:
	builtin unalias -m '[^+]*'

And separately, what's the use-case for zcompiling test-data files (!)?

[ericbn]

@danielshahaf, good points.

Should we change the test to do only:

BUFFER='cd;ls'

expected_region_highlight=(
  "1 2 builtin" # cd
  "4 5 command" # ls
)

without the functions? There's also a comment at the end of the test I quite don't understand:

# ... cd() and ls() should still be a functions
# when _zsh_highlight runs.  Leaving the wrapper functions is harmless.

And it makes no sense to compile the test-data. I'm just compiling everything under highlighters. I should fix that on my side...

[danielshahaf]

No, we shouldn't. The whole point of the test is to test that a function that shadows a builtin (or a command) is highlighted as a function.

I've fixed the comment in 5436d3e, thanks for pointing it out.

Does this address everything?

[ericbn]

Does this address everything?

Isn't function ls preferred over ls(), to address the issue when ls is already an alias...?

[danielshahaf]

Aliases typically change the default behaviour (e.g., alias ls='ls -F') in ways that may break scripts. The right answer is for .zshrc aliases not to be in effect while running the test suite.

That's already the case when the test suite is launched by make test.

See also #444.

[danielshahaf]

Aliases typically change the default behaviour (e.g., alias ls='ls -F') in ways that may break scripts.

To be clear here, I refer to scripts that run ls, e.g., if ls | grep '^foo$'; then …, with no functions involved.

[ericbn]

Sounds good. Closing this. Not compiling the files under test-data works for me, and is also an optimization. Thank you!

[danielshahaf]

You're welcome. Thanks for the report!