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

Expose null_literal as a node #139

Merged
merged 2 commits into from
Aug 18, 2024
Merged

Conversation

afroozeh
Copy link
Contributor

@afroozeh afroozeh commented Aug 3, 2024

We need to expose null literals in parse trees, otherwise, it will not be possible to syntactically differentiate between some constructs. Example: currently, these two functions yield the same parse tree:

fun f() = null
fun f() { }
source_file
  function_declaration
    simple_identifier
    function_value_parameters
    function_body

With the changes in this PR, the first function will have a null_literal in the tree:

source_file
  function_declaration
    simple_identifier
    function_value_parameters
    function_body
      null_literal

Fixes #140

We need to expose null literals in parse trees, otherwise, it will not
be possible to syntactically differentiate between some constructs.
Example: currently, these two functions yield the same parse tree:
```
fun f() = null
fun f() { }
```
```
source_file
  function_declaration
    simple_identifier
    function_value_parameters
    function_body
```

With the changes in this PR, the first function will have a
`null_literal` in the tree:
```
source_file
  function_declaration
    simple_identifier
    function_value_parameters
    function_body
      null_literal
```
@github-actions github-actions bot added the grammar Related to the grammar label Aug 3, 2024
@afroozeh afroozeh mentioned this pull request Aug 5, 2024
@afroozeh
Copy link
Contributor Author

afroozeh commented Aug 5, 2024

@fwcd Can you please check this PR? I have some other PRs that are stacked and like to merge them. I really don't want to fork the work on tree-sitter-kotlin, but expect some faster response time from you.
Also, it seems like someone posted an exact duplicate of this PR: #141
It doesn't matter for me which one gets merged, but let's merge one of them and move on with this one.

@wolfmanstout
Copy link
Contributor

@fwcd Can you please check this PR? I have some other PRs that are stacked and like to merge them. I really don't want to fork the work on tree-sitter-kotlin, but expect some faster response time from you.

Also, it seems like someone posted an exact duplicate of this PR: #141

It doesn't matter for me which one gets merged, but let's merge one of them and move on with this one.

+1 I just closed my dupe PR. I hadn't seen this one when I made that but it's very similar.

@afroozeh
Copy link
Contributor Author

afroozeh commented Aug 5, 2024

@wolfmanstout please check the new commit. It adds the changes from your PR to this one.

@wolfmanstout
Copy link
Contributor

@wolfmanstout please check the new commit. It adds the changes from your PR to this one.

Looks great, thanks.

@wolfmanstout
Copy link
Contributor

@wolfmanstout please check the new commit. It adds the changes from your PR to this one.

Please also update the PR description to include "Fixes #140".

@fwcd fwcd merged commit 6880d15 into fwcd:main Aug 18, 2024
2 checks passed
@fwcd
Copy link
Owner

fwcd commented Aug 18, 2024

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
grammar Related to the grammar
Projects
None yet
Development

Successfully merging this pull request may close these issues.

null should correspond to a node
3 participants