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

Exclude carriage return from comment nodes. #45

Merged
merged 5 commits into from
Jun 14, 2024

Conversation

wolfmanstout
Copy link
Contributor

@wolfmanstout wolfmanstout commented Nov 24, 2023

Fixes #36

@wolfmanstout
Copy link
Contributor Author

wolfmanstout commented Nov 24, 2023

One thing that was surprising (to me) about the Mocha unit tests: I have to run "npm install" twice to reliably rebuild after grammar changes (you can bet I found out the hard way, after nearly tearing my hair out). This is because npm install rebuilds the bindings based on binding.gyp as its first step, before parser.c is regenerated (which is a dependency of the bindings). If someone who is more familiar with npm and/or tree-sitter knows if there's a better way, that would be helpful. Otherwise this might be worth documenting.

@wolfmanstout
Copy link
Contributor Author

I fixed the aforementioned issue (following tree-sitter-python here).

@wenkokke
Copy link
Owner

Thanks!

Any reason for choosing Mocha?

@wolfmanstout
Copy link
Contributor Author

I saw Cursorless uses it 😂

I'm new to Node.js so no strong preference. I see that the most recent version of Node.js released this year has a native lightweight framework we could also use. Not sure if tooling support is as good, and means people would need recent Node to run tests, though.

@wenkokke
Copy link
Owner

Let's not depend on the latest version.

For now, I'd suggest jest?

@wenkokke
Copy link
Owner

Paging @pokey: Why Mocha over Jest?

@wenkokke
Copy link
Owner

Should make a note that if we add a testing framework, we should port the current JS tests to it.

@pokey
Copy link
Contributor

pokey commented Nov 25, 2023

Yeah we'd prob be using jest if it weren't for mocha being default framework for VSCode tests

@wolfmanstout
Copy link
Contributor Author

No problem, I can switch to Jest. Any reason for the preference? Just curious for my own education.

@wenkokke
Copy link
Owner

wenkokke commented Nov 26, 2023

  1. I know how to use it.
  2. It's more widely used.
  3. It's a little simpler.

@wolfmanstout
Copy link
Contributor Author

Updated to use Jest, ready for review.

@wolfmanstout wolfmanstout marked this pull request as ready for review November 26, 2023 07:38
@wolfmanstout
Copy link
Contributor Author

Any questions? Feel free to ping me on Slack.

@wenkokke wenkokke merged commit 2a2a6fd into wenkokke:dev Jun 14, 2024
3 checks passed
@wenkokke
Copy link
Owner

This is now merged. Sorry for the delay!

@wolfmanstout wolfmanstout deleted the fix_comment_carriage_return branch June 16, 2024 05:13
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.

Comment node includes trailing \r
3 participants