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

Fix HurlRunnterToEntry using treesitter #116

Merged
merged 2 commits into from
Mar 11, 2024

Conversation

hsanson
Copy link
Contributor

@hsanson hsanson commented Mar 10, 2024

WHAT

Modify HurlRunnerToEntry to work using treesitter parser. This allows to run to a specific entry without having to be exactly located at the start of the entry where the verb is located.

WHY

HOW

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • Linter
  • Tests
  • Review comments
  • Security

Copy link

changeset-bot bot commented Mar 10, 2024

⚠️ No Changeset found

Latest commit: ab99779

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link
Contributor

sweep-ai bot commented Mar 10, 2024

Apply Sweep Rules to your PR?

  • Apply: All docstrings and comments should be up to date.
  • Apply: Commit messages should be descriptive and provide a clear explanation of the changes made.
  • Apply: Author names should be consistent and use proper formatting.
  • Apply: Code should be properly formatted and follow a consistent style guide.
  • Apply: There should be no unnecessary whitespace or trailing spaces in the code.
  • Apply: Variable and function names should be meaningful and descriptive.

This is an automated message generated by Sweep AI.

@jellydn
Copy link
Owner

jellydn commented Mar 10, 2024

Thanks. I will take a try tonight.

@jellydn jellydn merged commit 5bbc6cd into jellydn:main Mar 11, 2024
7 checks passed
@jellydn
Copy link
Owner

jellydn commented Mar 11, 2024

FYI - Just synced your change to https://github.com/jellydn/hurl.nvim/tree/canary and the PR is not working properly yet.

@hsanson
Copy link
Contributor Author

hsanson commented Mar 11, 2024

FYI - Just synced your change to https://github.com/jellydn/hurl.nvim/tree/canary and the PR is not working properly yet.

Please let me know what issues you are experiencing and if possible a sample hurl file to reproduce them. I have tested it on my hurl files and seems to work properly.

@jellydn
Copy link
Owner

jellydn commented Mar 13, 2024

FYI - Just synced your change to canary and the PR is not working properly yet.

Please let me know what issues you are experiencing and if possible a sample hurl file to reproduce them. I have tested it on my hurl files and seems to work properly.

Sure, I will push my test file to repository so you could try as well.

@jellydn
Copy link
Owner

jellydn commented Mar 13, 2024

Hi @hsanson Here is the simple test file ae57eba It works with main but not working properly with canary branch.

@jellydn
Copy link
Owner

jellydn commented Mar 13, 2024

Just FYI - I've reverted to using the find verb instead of treesitter parser.

Reproduce:

  • Checkout canary, open example.hurl
  • Place cursor at line 1, then run HurlRunnerAt
image

The canary branch will return the Google response instead of manga API. @hsanson

@jellydn
Copy link
Owner

jellydn commented Mar 13, 2024

Canary:

image

Main:
image

@hsanson
Copy link
Contributor Author

hsanson commented Mar 13, 2024

@jellydn sorry for the issue... please test if this fixes it for you:

jellydn added a commit that referenced this pull request Mar 14, 2024
* Fix HurlRunnterToEntry using treesitter (#116)

* chore(doc): auto generate docs

* chore(test): add API tests for dog breeds

* Fix find entry via treesitter (#124)

* Fix HurlRunnterToEntry using treesitter

* chore(doc): auto generate docs

* Fix find entry method returing two nodes range.

Lua index starts on 1 so we were accidentally returing the line range of
the current node plus one line of the next node.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* refactor(http_utils): make find_hurl_entry_positions_in_buffer a module function

---------

Co-authored-by: Horacio Sanson <[email protected]>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
@jellydn
Copy link
Owner

jellydn commented Mar 14, 2024

Thanks @hsanson Almost there. The parser logic will failed if there is a comment on hurl, e.g: 9f773ef

@jellydn
Copy link
Owner

jellydn commented Mar 14, 2024

For now, this is my solution :) 4d4e4ff

@hsanson
Copy link
Contributor Author

hsanson commented Mar 15, 2024

@jellydn simple solution and seems to work as far as I can see... thanks. Better than trying to fix the treesitter parser that does not seem to properly parse the comments and treat them as errors. Specially the top most comment.

You can see how the treesitter sees the file using :InspectTree. This is a very useful tool to understand hurl grammar.

@jellydn
Copy link
Owner

jellydn commented Mar 15, 2024

Great. Thanks, @hsanson Also, the treesitter parser only works for nightly. I will bring the find verb back to fallback on stable version.

@jellydn
Copy link
Owner

jellydn commented Mar 15, 2024

FYI c10a905

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.

2 participants