-
Notifications
You must be signed in to change notification settings - Fork 14
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
docs: extend Javadoc #90
base: master
Are you sure you want to change the base?
docs: extend Javadoc #90
Conversation
@@ -40,6 +40,7 @@ Charset charset() { | |||
* @param charset one of {@link StandardCharsets#UTF_8}, {@link StandardCharsets#UTF_16BE}, | |||
* {@link StandardCharsets#UTF_16LE}, or {@link StandardCharsets#UTF_16} (native byte order). | |||
* @throws IllegalArgumentException If the character set is invalid. | |||
* @since 0.25.1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See #85; for external users this method effectively did not exist before. This assumes the next version will be 0.25.1
@@ -67,6 +67,7 @@ private static UnsatisfiedLinkError unresolved(String name) { | |||
* <strong>The {@linkplain Arena} used to load the language | |||
* must not be closed while the language is being used.</strong> | |||
* | |||
* @throws UnsatisfiedLinkError If the language symbol could not be found. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thrown by the unresolved(language)
call
* @param progressCallback Called when parsing progress was made. Return {@code true} to cancel parsing, | ||
* {@code false} to continue parsing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I hope I understood the logic in https://github.com/tree-sitter/tree-sitter/blob/37a9ecd5b7d52786d0673611f0b8fc54292b1ac1/lib/src/parser.c#L1553 correctly, and true
does actually cancel parsing.
That is probably worth documenting, otherwise users might be unsure how this progressCallback
works, or assume that true
means "continue parsing".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But that true
means cancelling is maybe a bit confusing (even if that matches the tree-sitter behavior). Maybe it would be better to rename it to cancellationCallback
(see also tree-sitter/tree-sitter#4312).
It seems even ParserTest
might have gotten this wrong?
var options = new Parser.Options((state) -> state.getCurrentByteOffset() <= 1000); |
Maybe that should be
>=
instead of <=
to cancel as soon as 1000 or more bytes were consumed? Or is the intention here to additionally verify that the parser consumes the bytes in pieces and not all the 1024 bytes at once?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is probably worth documenting, otherwise users might be unsure how this
progressCallback
works, or assume thattrue
means "continue parsing".
That is what I assumed. 💀
* @param progressCallback Progress handler. Return {@code true} to cancel query execution, | ||
* {@code false} to continue query execution. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as the comment in Parser
; I hope I understood the logic in https://github.com/tree-sitter/tree-sitter/blob/37a9ecd5b7d52786d0673611f0b8fc54292b1ac1/lib/src/query.c#L3631 correctly, and true
does actually cancel query execution.
Build probably fails because the |
* @param progressCallback Called when parsing progress was made. Return {@code true} to cancel parsing, | ||
* {@code false} to continue parsing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is probably worth documenting, otherwise users might be unsure how this
progressCallback
works, or assume thattrue
means "continue parsing".
That is what I assumed. 💀
src/main/java/io/github/treesitter/jtreesitter/LookaheadIterator.java
Outdated
Show resolved
Hide resolved
|
||
Java bindings to the [tree-sitter] parsing library. | ||
|
||
## Usage |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems superfluous. Just click the badge.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My idea here was to make it a bit easier for users to discover.
Otherwise new users might not know how to get started with this project, and might not consider looking at the Javadoc (in case they do notice the badge). They might not expect such an extensive usage example in the package Javadoc because not all projects have that (respectively have it in separate documentation or in the README).
…/documentation-improvements
Tries to improve the documentation a bit. If you want anything changed, please let me know or feel free to adjust it yourself (pull request has "Allow edits" enabled).