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

update font rules, indention, faces and readme #68

Merged
merged 13 commits into from
Jun 14, 2024

Conversation

craynot
Copy link
Contributor

@craynot craynot commented May 21, 2024

Hi!

This merge fixes compitability with latest tree-sitter-php, adds some font rules, updates doc and fixes indention

1. Fixes compitability with latest tree-sitter-php
In latest versions node string_value has been changed to string_content

2. Add fontifications to base class and ? symbol in optional types
(base_clause (name) @php-class)
(optional_type "?" @php-type)

3. Add fontification to $this
Looks like $ symbol is not allowed in capture-name's in queries, so related faces was renamed. To avoid conflicts, where php-face is already loaded from php-mode, those faces was moved to another file php-ts-face.el
Also there was a problem with overriding fonts by rules for all others variables and sigils, so this rules was moved to separate feature. If there is a cleaner way to do this, please let me know.

4. Fix indention of open bracket
Indention of open bracket in method declaration did not work, it was fixed by adding
(parent-is "method_declaration") parent-bol 0) to php-ts-mode--indent-rules

5. Update README
Added simple installation & configuration tips.

6. Fix indention of default expression in switch-case
Correspondent test was added

7. Add face tests
This is my first experience with Eask, so maybe there is a better way to test font faces.
And maybe it is a good idea to split face test to different erts files for each feature. Having one big file or 15 small files, what is better is debatable

8. Add some font rules
Mostly it is about qualified_name, for each case test was added

9. Add built-in feature
To highlight built-in functions, like in php-mode

10. Fix indention of match statement

@piotrkwiecinski
Copy link
Contributor

Hi @craynot,
thank you for submitting the pull request. Overall it looks good.

As mentioned in the README there are plans to transfer this project to GNU Emacs, so if you haven't assigned a copyright to the FSF yet, please refer to the Org-mode contribute guide and send an email to Kenta: tadsan@zonu.me

@@ -77,6 +78,7 @@
;; "compound_statement" contains the body of many statements.
;; For example function_definition, foreach_statement, etc.
((parent-is "compound_statement") parent-bol ,offset)
((parent-is "method_declaration") parent-bol 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

Would you be able to add a test for this case?
We have very basic suite here: https://github.com/emacs-php/php-ts-mode/blob/master/tests/php-ts-mode-resources/indent.erts

It would be great to increase the coverage while we fix issues.

@piotrkwiecinski piotrkwiecinski mentioned this pull request May 21, 2024
19 tasks
@craynot
Copy link
Contributor Author

craynot commented May 21, 2024

Hi @piotrkwiecinski
Thank you for the reply!

I have added test for this case.

Can you clarify about FSF? I have send the request to assign@gnu.org, now waiting for the reply. Or should i have send it to tadsan@zonu.me ?

@piotrkwiecinski
Copy link
Contributor

Hi @craynot.
FSF is going to send you a form to sign.
Once you sign it, send it back to FSF.
They'll send you the final copy signed by them.
Please send the final copy to tadsan@zonu.me .

It may take some time but you have to sign it only one time.

@craynot
Copy link
Contributor Author

craynot commented May 21, 2024

Thank you @piotrkwiecinski

php-ts-face.el Outdated Show resolved Hide resolved
php-ts-mode.el Outdated Show resolved Hide resolved
@piotrkwiecinski
Copy link
Contributor

Nice rainbow 😄

php-ts-rainbow

@piotrkwiecinski
Copy link
Contributor

Great work @craynot. As soon as we have signed FSF doc it's fine to merge @zonuexe.

Copy link
Member

@zonuexe zonuexe left a comment

Choose a reason for hiding this comment

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

I've received a copy of the FSF copyright assignment from @craynot so we can merge this PR. Thanks for all your help!

@zonuexe zonuexe merged commit 461bc02 into emacs-php:master Jun 14, 2024
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.

3 participants