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: parse doc strings with litrs #1015

Merged
merged 1 commit into from
Feb 2, 2025
Merged

Conversation

adalinesimonian
Copy link
Contributor

Fixes #885

Screenshot

@lilizoey lilizoey added bug c: register Register classes, functions and other symbols to GDScript labels Jan 18, 2025
@lilizoey
Copy link
Member

is there a way you could add a test for this case to verify that it's fixed?

@adalinesimonian
Copy link
Contributor Author

adalinesimonian commented Jan 18, 2025

register_docs_test.rs checks the formatting and would catch this case, the test just needs to be updated since right now it would fail expecting a ´\"´. I'm changing the test in PR #1017 so there may be a conflict between these two PRs, I can update the test here and then resolve the conflict in the other branch after this one is merged given that this one is small and easier to review, or I could do it the other way and leave this branch until the other one is merged and then rebase onto main and patch the fix. Any preference one way or another?

@GodotRust
Copy link

API docs are being generated and will be shortly available at: https://godot-rust.github.io/docs/gdext/pr-1015

@Bromeon
Copy link
Member

Bromeon commented Feb 1, 2025

Now that #1017 has been merged, what's your plan with this?
If you'd like to resume work, could you add a test case? 🙂

@adalinesimonian
Copy link
Contributor Author

adalinesimonian commented Feb 1, 2025

Now that #1017 has been merged, what's your plan with this? If you'd like to resume work, could you add a test case? 🙂

Now sounds like a great time to rebase this onto main. Re: test case, does updating the docs snapshot suffice? It has a doc string literal with this same issue, so by updating that snapshot if this ever stops working we'll catch it in that existing test.

@adalinesimonian adalinesimonian force-pushed the litrs branch 4 times, most recently from 0be6c39 to 0cd8b20 Compare February 1, 2025 19:34
@Bromeon
Copy link
Member

Bromeon commented Feb 1, 2025

Re: test case, does updating the docs snapshot suffice?

Yes, definitely! Thanks also for the link 👍

@Bromeon
Copy link
Member

Bromeon commented Feb 2, 2025

Thanks a lot!

Merged via the queue into godot-rust:master with commit d7d2de6 Feb 2, 2025
15 checks passed
@adalinesimonian adalinesimonian deleted the litrs branch February 2, 2025 09:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug c: register Register classes, functions and other symbols to GDScript
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Quotes from doc comments are unnecessarily escaped
4 participants