Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

syntax: don't pick up chan chains (channel of channels) #678

Merged
merged 1 commit into from
Jan 3, 2016

Conversation

fatih
Copy link
Owner

@fatih fatih commented Jan 3, 2016

Fixes the following wrong highlighting:

image

into:

image

This is a very weird error. We want to show an error if the chan
statement is in form of:

<- chan
chan <-

The correct form should be of course:

<-chan
chan<-

But for example, if we have a channel of channels (like below), the
selected piece of whitespace is also highlighted as an error

chan<- chan<-
      ^

Because it's the part of <- chan, whereas we should not treat as an
error because we have two chan<- and chan<- statements.

To fix this, we add a negative lookbehind regex statement. It will not
match a text if it's preceded with other text.

First let assemble the current regex for the case: <- chan
(receive-only annotation), which is:

\(<-\)\@<=\s\+\(chan\>\)\@=

The first part is:

\(<-\)\@<=\s\+\

Here the pattern @<= matches with zero width if the preceding atom
matches just before what follows. \s\+ means white space of at least
one character. So it matches whitespaces which precedes the
statement <-

Now the second part is only useful with the first part. I've added the
placeholder to make it more readable:

PLACEHOLDER(chan\>\)\@="

First the pattern \@= means to match the preceding atom with zero
width. So it will match the PLACEHOLDER till it finds a chan
statement. Once it finds it, it doesn't proceed anymore. So we only
highlight the whitespaces between <- and chan only if we have a
white space of at least one character

Finally we can introduce our fix now. We're going to pick the <-
statemetn if it doesn't follow a chan. So chan<- should NOT trigger
an error anymore, because we're not going to pick arrows in that form.

To match an arrow which doesn't follow a chan statement can be done via
a negative lookbehind, which is in the form of:

\(chan\)\@<!<-

This literally means: "Match <- only if it doesn't follow chan.
We're going to use this pattern for the plain <- pattern. Not we're
just going to replace this into our current regex:

This original regex:

\(<-\)\@<=\s\+\(chan\>\)\@=

will be changed into:

\(\(chan\)\@<!<-\)\@<=\s\+\(chan\>\)\@=

Fixes #655

@fxkr
Copy link

fxkr commented Jan 3, 2016

FTR: this introduces a regression; more details in my comment in #655

@fatih
Copy link
Owner Author

fatih commented Jan 3, 2016

Hi @fxkr Thanks for testing it! I've force pushed the changes you mentioned. Can you please try again?

@fxkr
Copy link

fxkr commented Jan 3, 2016

  • You may want to rebase it onto the current master.
  • The commit message is outdated (specifically, the regex in the last line of the commit message.
  • There's the extra f at the end of the last regex you touched, I'm sure that breaks something. Sorry that I didn't notice before.
-  syn match goSpaceError display "\(\(^\|[={(,;]\)\s*<-\)\@<=\s\+"
+  syn match goSpaceError display "\(\(^\|[={(,;]\)\s*<-\)\@<=\s\+f"

This is a very weird error. We want to show an error if the chan
statement is in form of:

	<- chan
	chan <-

The correct form should be of course:

	<-chan
	chan<-

But for example, if we have a channel of channels (like below), the
selected piece of whitespace is also highlighted as an error

	chan<- chan<-
	      ^

Because it's the part of `<- chan`, whereas we should not treat as an
error because we have two `chan<-` and `chan<-` statements.

To fix this, we add a `negative lookbehind` regex statement. It will not
match a text if it's preceded with other text.

First let assemble the current regex for the case: `<- chan`
(receive-only annotation), which is:

	\(<-\)\@<=\s\+\(chan\>\)\@=

The first part is:

	\(<-\)\@<=\s\+\

Here the pattern \@<= matches with zero width if the preceding atom
matches just before what follows. `\s\+` means white space of at least
one character. So it matches **whitespaces** which precedes the
statement `<-`

Now the second part is only useful with the first part. I've added the
placeholder to make it more readable:

	PLACEHOLDER(chan\>\)\@="

First the pattern `\@=` means to match the preceding atom with zero
width. So it will match the **PLACEHOLDER** till it finds a `chan`
statement. Once it finds it, it doesn't proceed anymore. So we only
highlight the whitespaces between `<-` and `chan` only if we have a
white space of at least one character

Finally we can introduce our fix now. We're going to pick the `<-`
statemetn if it doesn't follow a `chan`. So `chan<-` should NOT trigger
an error anymore, because we're not going to pick arrows in that form.

To match an arrow which doesn't follow a chan statement can be done via
a `negative lookbehind`, which is in the form of:

	\(\<chan\>\)\@<!<-

This literally means: "Match `<-` only if it doesn't follow `chan`.
We're going to use this pattern for the plain `<-` pattern. We also fix
the issue of only matching `chan` and not words like `mychan`. Aslo note
we're just going to replace this into our current regex:

This original regex:

	\(<-\)\@<=\s\+\(\<chan\>\)\@=

will be changed into:

	\(\(\<chan\>\)\@<!<-\)\@<=\s\+\(\<chan\>\)\@=

Fixes #655
@fatih
Copy link
Owner Author

fatih commented Jan 3, 2016

@fxkr fixed. Nice catch, seems I've accidentally added it.

fatih added a commit that referenced this pull request Jan 3, 2016
syntax: don't pick up chan chains (channel of channels)
@fatih fatih merged commit b26351b into master Jan 3, 2016
@fatih
Copy link
Owner Author

fatih commented Jan 3, 2016

Thanks @fxkr for the feedback. Please open a issue if you find other cases.

@fatih fatih deleted the fix-chan-highlighting branch January 3, 2016 20:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Not a spacing error: "<-chan <-chan"
2 participants