Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

Add scope for PHP primitive types #389

Closed
wants to merge 16 commits into from
Closed

Add scope for PHP primitive types #389

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
4d1ef95
Add `storage.primitive.php` scope for PHP primitive types
bakerkretzmar Apr 15, 2020
688c886
Add primitive scope to typed properties and update tests
bakerkretzmar Apr 15, 2020
92e6bbf
Add primitive scope to closure return types and update tests
bakerkretzmar Apr 15, 2020
6478a28
Add primitive scope to arrow function return types and update tests
bakerkretzmar Apr 15, 2020
2a938ff
Add primitive scope to function return types and update tests
bakerkretzmar Apr 15, 2020
17e2c45
Add primitive scope to type casts and update tests
bakerkretzmar Apr 15, 2020
7206557
Add primitive scope to function and method argument typehints and upd…
bakerkretzmar Apr 15, 2020
484b025
Update more tests
bakerkretzmar Apr 15, 2020
91373a0
Add primitive scope to array and callable argument typehints and upda…
bakerkretzmar Apr 15, 2020
e818c0a
Update scope to `storage.type.primitive.php`
bakerkretzmar Apr 15, 2020
1bbb5a0
Add word boundaries to regex
bakerkretzmar Apr 15, 2020
261e693
Fix missing quotes on keys
bakerkretzmar Apr 29, 2020
d1071b8
Consistent quotes in tests
bakerkretzmar Apr 29, 2020
915ff09
Replace \b with ^...$ in regex
bakerkretzmar Apr 29, 2020
a097719
Remove parentheses from regex
bakerkretzmar Apr 29, 2020
054d564
Revert to using \b in regex
bakerkretzmar Apr 30, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 42 additions & 0 deletions grammars/php.cson
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -462,6 +462,11 @@
'name': 'keyword.operator.nullable-type.php'
'3':
'name': 'storage.type.php'
'patterns': [
{
'include': '#primitives'
}
]
}
]
}
Expand Down Expand Up @@ -503,6 +508,11 @@
'name': 'keyword.operator.nullable-type.php'
'3':
'name': 'storage.type.php'
'patterns': [
{
'include': '#primitives'
}
]
}
]
}
Expand Down Expand Up @@ -546,6 +556,11 @@
'name': 'keyword.operator.nullable-type.php'
'4':
'name': 'storage.type.php'
'patterns': [
{
'include': '#primitives'
}
]
'name': 'meta.function.php'
'patterns': [
{
Expand Down Expand Up @@ -585,6 +600,11 @@
]
'4':
'name': 'storage.type.php'
'patterns': [
{
'include': '#primitives'
}
]
'5':
'name': 'variable.other.php'
'6':
Expand Down Expand Up @@ -638,6 +658,11 @@
'name': 'punctuation.definition.storage-type.begin.bracket.round.php'
'2':
'name': 'storage.type.php'
'patterns': [
{
'include': '#primitives'
}
]
'3':
'name': 'punctuation.definition.storage-type.end.bracket.round.php'
}
Expand Down Expand Up @@ -825,6 +850,13 @@
}
]
'repository':
'primitives':
'patterns': [
{
'match': '\\b(array|bool|callable|float|int|iterable|object|string)\\b'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have not tried this (especially in VS Code), but since you are only using this for nested matching, you could try using ^ and $ instead of the two \\b. If it works exactly as before, you should consider using that form instead.

While \b is mostly "fine", it's somewhat tricky thing to get right and sets a poor example to others. I cannot think of specific counterexamples from top of my head, but historically we have had to fix poor usages of \b here, which is why I suggest avoiding it altogether whenever the replacement is trivial to understand.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Ingramz I'm not sure how unicode is treated in vscode, but in atom I couldn't abuse this with stuff like stringʕ•ᴥ•ʔ. However, this might be a problem in vscode or some other editor.

Although it's rather edge case, replacing \b with ^...$ is a good idea.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One possible example is using emojis like int🐱.

Copy link
Author

@bakerkretzmar bakerkretzmar Apr 29, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I basically copy-pasted from other parts of the syntax to start with, so I appreciate this advice.

All the tests pass with ^(...)$ but interestingly, this doesn't work in VS Code. No clue why, it just doesn't tokenize properly. But if I remove the capturing group, so it really is just ^...$ (no parentheses) then it works as expected.

Is there any reason to keep those, or is removing them as I've done now fine?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without parentheses you have equivalent of: (^array)|bool|callable|float|int|iterable|object|(string$) so only array and string are broken then.

Okay I dived into this and found cause microsoft/vscode-textmate#74 tl;dr only $ is working somewhat correctly and all because substring(line start, capture end) is used for match, thus any start anchors are useless there.

Even if ^..$ are better, implementation makes them useless in vscode.

Copy link
Author

@bakerkretzmar bakerkretzmar Apr 30, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting. So would something like (^array$)|(^bool$)|(^callable$)|... work here? Or would it still break because ^ doesn't work properly at all?

@Ingramz it looks like \\b might be simpler after all if that's okay with you! 😂

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, ^ is broken. And yes, in that case \b is better. I wasn't aware of that until now and this explains some weird behaviors which I had in the past.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since it is broken in vscode then we have no other choice than to use \b. Thank you for trying it out and @KapitanOczywisty for finding the relevant issue. Rest of the pull request looks great now. I am however annoyed by this vscode's deviation from spec.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that's frustrating. I've changed it back to \b.

'name': 'storage.type.primitive.php'
}
]
'class-builtin':
'patterns': [
{
Expand Down Expand Up @@ -1267,6 +1299,11 @@
'name': 'keyword.operator.nullable-type.php'
'2':
'name': 'storage.type.php'
'patterns': [
{
'include': '#primitives'
}
]
'3':
'name': 'variable.other.php'
'4':
Expand Down Expand Up @@ -1314,6 +1351,11 @@
]
'3':
'name': 'storage.type.php'
'patterns': [
{
'include': '#primitives'
}
]
'4':
'name': 'variable.other.php'
'5':
Expand Down
Loading