StanAngeloff / php.vim

An up-to-date Vim syntax for PHP (7.x supported)
477 stars 69 forks source link

Distinguish PHPDoc annotation arguments clearly #19

Closed commonquail closed 10 years ago

commonquail commented 10 years ago

The PHPDoc annotations match the current line greedily. In addition to this making annotation arguments (e.g. type specifiers) blend in with the rest of the line it is inconsistent with any continuation lines, which take the default comment colour. This makes the PHPDoc matching a little more sophisticated, so annotation arguments and identifiers stand out.

Actual vs. expected: phpdocparam

This can arguably be simplified. In particular, line 18 in the screenshot takes steps to render the commas as comments rather than part of the identifier or annotation name. Since it's insane to use commas in identifier names, that complexity may not be necessary. A similar case can be made for line 21.

commonquail commented 10 years ago

Let me explain the behaviour I desired:

You will note that this is not exactly what this changeset does. The main reason for that is that I decided the change would be too comprehensive for the gain and that the complexity wasn't worth it, but if you disagree I'd be willing to (try and) carry it out.

The first two points are most important to me. The rest express ideals that, practically speaking, do not seem necessary. This is a naive implementation that solves most of my problems, and since I wage a religious war on any @author tags I see in codebases I work on, the fact that it looks silly does not concern me.

For an alternative, more robust solution, I would probably partition tags into sets with formally definable parameters (@throws, @param) and no formally definable parameters (@author). The latter would apply no special matching to arguments. The former would have to distinguish between tags that expect only a type (@throws, @return) and tags that expect an identifier with an optional type (@param).

An alternative-alternative, less intrusive change, would seek to adjust phpDocTags and phpDocParam in such a way that they can contain at most one optional type and one optional identifier. I actually prefer that to what I did but there doesn't seem to be a way to express that a named pattern should follow directly after another pattern (e.g. /phpDocParam?phpDocIdentifier?/).

StanAngeloff commented 10 years ago

Thank you for taking the time to explain this more clearly. I have now merged your pull request.

I agree, the doc block handling code requires a bit of attention and your ideas are a good starting point.

An alternative-alternative, less intrusive change, would seek to adjust phpDocTags and phpDocParam in such a way that they can contain at most one optional type and one optional identifier. I actually prefer that to what I did but there doesn't seem to be a way to express that a named pattern should follow directly after another pattern (e.g. /phpDocParam?phpDocIdentifier?/).

Can nextgroup take care of this? We use it for implements and extends so that they match only if followed by a class name, etc.

commonquail commented 10 years ago

It looks like it might. I can try that out but I might not have time until next week.