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.

Sign up for GitHub

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, and strip, comments (#, // or /*) #2548

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Allow, and strip, comments (#, // or /*) #2548

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e659a5e
Allow, and strip, JSON comments (#, // or /*)
liquidaty Mar 2, 2023
8b479ca
Add --strip-comments option
liquidaty Jul 7, 2023
334d15c
update scanner to, while scanning comments, return 0 instead of OK
liquidaty Jul 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions src/jv_parse.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -649,7 +649,7 @@ static int stream_is_top_num(struct jv_parser* p) {
static pfunc scan_line_comment(struct jv_parser* p, char ch, jv* out) {
if(ch == '\n')
p->scan = scan_json;
return OK;
return 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we have some commentary in the commit message about why this change?

Copy link
Member

@wader wader Feb 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trying to understand the 0 -> OK change. OK is static const presult OK = "output produced"; so i guess something else must have changed before the change? only reference i can find is in #2548 (comment). Will digg more later.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I see that, but I'm not sure why that's needed, and anyways, there's places where we look for OK that need to be changed correspondingly.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

iirc the reason for this is fairly straightforward, which is that in the below block of jv_parse.c:

jv jv_parser_next(struct jv_parser* p) {
...
  while (!msg && p->curr_buf_pos < p->curr_buf_length) {
    ...
    msg = scan(p, ch, &value);
  }

the desired behavior, when a comment is encountered, is to simply ignore it and keep on going. Therefore the while loop must keep on going, which requires that !msg is true. Any other result will break the while loop, which would be incorrect.

That said, given that the return type of scan() is a pointer, it might be better for the scan_xxx() functions to all return NULL where they currently are returning of 0.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@liquidaty thanks for the explanation. think i tried to return OK instead when played around a bit with the code and pretty sure it also worked for some reason, was confusing

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it would be useful to define IGNORE_AND_CONTINUE as a presult equal to NULL (or alternatively, not null, with other corresponding changes e.g. !msg becomes msg == IGNORE_AND_CONTINUE)

}

static pfunc scan_c_comment_close(struct jv_parser* p, char ch, jv* out) {
Expand All @@ -658,24 +658,24 @@ static pfunc scan_c_comment_close(struct jv_parser* p, char ch, jv* out) {
} else {
p->scan = scan_c_comment;
}
return OK;
return 0;
}

static pfunc scan_c_comment(struct jv_parser* p, char ch, jv* out) {
if(ch == '*') {
p->scan = scan_c_comment_close;
}
return OK;
return 0;
}

static pfunc scan_slash_comment(struct jv_parser* p, char ch, jv* out) {
if(ch == '/') {
p->scan = scan_line_comment;
return OK;
return 0;
}
if(ch == '*') {
p->scan = scan_c_comment;
return OK;
return 0;
}
return "Incomplete comment token; slash must be followed by another slash or asterisk";
}
Expand Down Expand Up @@ -704,7 +704,7 @@ static pfunc scan_json(struct jv_parser* p, char ch, jv* out) {
if (p->st == JV_PARSER_NORMAL) {
if(ch == '/' && (p->flags & JV_PARSE_STRIP_COMMENTS)) {
p->scan = scan_slash_comment;
return OK;
return answer;
}

chclass cls = classify(ch);
Expand Down