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

Allow the visitor to abort or cease recursion #30

Open
connor4312 opened this issue Jan 8, 2020 · 8 comments
Open

Allow the visitor to abort or cease recursion #30

connor4312 opened this issue Jan 8, 2020 · 8 comments
Assignees
Labels
feature-request Request for new features or functionality help wanted Issues identified as good community contribution opportunities
Milestone

Comments

@connor4312
Copy link
Member

I'm working on a code lens that should decorate npm scripts. For this, I'm parsing a JSON file, and only want the top level "scripts" key. I'm entirely uninterested in other properties, and once I have that I'm happy to abort parsing.

It'd be nice to be able to return a value from JSONVisitor methods which could indicate that we want to cease recursion on the current node, or abort parsing entirely.

@aeschli
Copy link
Contributor

aeschli commented Jan 9, 2020

To abort, throw an exception and catch it in a try/catch around parse.

To cease recursion, we could add a return value on onObjectBegin and onArrayBegin. If you want to make a PR, I'm happy to take it.

@aeschli aeschli added the feature-request Request for new features or functionality label Nov 13, 2020
@aeschli aeschli added this to the Backlog milestone Nov 13, 2020
@aeschli aeschli added the help wanted Issues identified as good community contribution opportunities label Nov 13, 2020
@Vbbab
Copy link

Vbbab commented Nov 1, 2023

Just wondering, how would this work? Would returning, say, false, from onObjectBegin prevent further parsing of the current object? In that case, would parseObject() in visit() simply return false?

@aeschli
Copy link
Contributor

aeschli commented Nov 1, 2023

The object will be parsed, just the visitor won't get any callbacks for any of the properties of the object
Internally, parseObject still return true or false, based on the validity of the input.

@Vbbab
Copy link

Vbbab commented Nov 2, 2023

Alright, so as per my understanding, if at the start of an object I return false (meaning don't give callbacks) for onObjectBegin, the ONLY other call I will get for that object is onObjectEnd, and nothing for the inside (not even onSeparator, onLiteralValue, etc.)?

Asking this as I'm actually currently trying to work on a PR to add this, and just want to make sure I understand things correctly.

Thanks!

@aeschli
Copy link
Contributor

aeschli commented Nov 3, 2023

Yes, correct.

@Vbbab
Copy link

Vbbab commented May 13, 2024

Alright, sorry that it's been quite a while - I've had to put this issue on the back burner for a bit due to a lack of time.

However, here is a quick update -- I'm currently working on testing an implementation for this feature I've got, but it seems like my IDE is throwing out a strange error under src/test/json.test.ts:

Specifically, on both L90 and L93:

onObjectBegin: noArgHalderWithPath('onObjectBegin'),
onObjectProperty: oneArgHalderWithPath('onObjectProperty'),
onObjectEnd: noArgHalder('onObjectEnd'),
onArrayBegin: noArgHalderWithPath('onArrayBegin'),

Shouldn't onObjectBegin and onArrayBegin normally be functions returning void? Yet the noArgHalderWithPath function returns just a number?? I'm quite confused as to how the testing mechanism is supposed to work here...

In addition, in src/impl/parser.ts, there are also some of what appear to be callback adapters:

function toNoArgVisit(visitFunction?: (offset: number, length: number, startLine: number, startCharacter: number) => void): () => void {
return visitFunction ? () => visitFunction(_scanner.getTokenOffset(), _scanner.getTokenLength(), _scanner.getTokenStartLine(), _scanner.getTokenStartCharacter()) : () => true;
}
function toNoArgVisitWithPath(visitFunction?: (offset: number, length: number, startLine: number, startCharacter: number, pathSupplier: () => JSONPath) => void): () => void {
return visitFunction ? () => visitFunction(_scanner.getTokenOffset(), _scanner.getTokenLength(), _scanner.getTokenStartLine(), _scanner.getTokenStartCharacter(), () => _jsonPath.slice()) : () => true;
}
function toOneArgVisit<T>(visitFunction?: (arg: T, offset: number, length: number, startLine: number, startCharacter: number) => void): (arg: T) => void {
return visitFunction ? (arg: T) => visitFunction(arg, _scanner.getTokenOffset(), _scanner.getTokenLength(), _scanner.getTokenStartLine(), _scanner.getTokenStartCharacter()) : () => true;
}
function toOneArgVisitWithPath<T>(visitFunction?: (arg: T, offset: number, length: number, startLine: number, startCharacter: number, pathSupplier: () => JSONPath) => void): (arg: T) => void {
return visitFunction ? (arg: T) => visitFunction(arg, _scanner.getTokenOffset(), _scanner.getTokenLength(), _scanner.getTokenStartLine(), _scanner.getTokenStartCharacter(), () => _jsonPath.slice()) : () => true;
}

According to the function signature, these should return functions of type () => void, yet if visitFunction is null or otherwise falsy, they all seem to return () => true, violating the signature. Is the entire point to prevent visitFunction from being omitted? If so, why make it optional? (My IDE doesn't error here though, so perhaps it's just that I don't understand TypeScript enough😅)

Sorry if all of this is a lot... Thanks, though! 😄

@aeschli
Copy link
Contributor

aeschli commented May 13, 2024

Shouldn't onObjectBegin and onArrayBegin normally be functions returning void, or define it to what you want.

Yes, that's just to keep the code compact.
You can write it as
let noArgHalder = (id: keyof JSONVisitor) => (offset: number, length: number, startLine: number, startCharacter: number) => { actuals.push({ id, text: input.substr(offset, length), startLine, startCharacter }); } ;

Yes, () => true should be () => {}

@Vbbab
Copy link

Vbbab commented May 14, 2024

Alright, I submitted a PR -- feel free to take a look!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request Request for new features or functionality help wanted Issues identified as good community contribution opportunities
Projects
None yet
Development

No branches or pull requests

3 participants