janet-lang / janet

A dynamic language and bytecode vm
https://janet-lang.org
MIT License
3.38k stars 217 forks source link

Discrepancy between the website docs and behavior for the `look` peg special #1465

Closed sogaiu closed 2 weeks ago

sogaiu commented 2 weeks ago

I noticed today that it's possible to use the peg special look with only one argument:

$ janet
Janet 1.35.2-fda0a081 linux/x64/gcc - '(doc)' for help
repl:1:> (peg/match '(sequence (look 2) (capture 1)) "a")
nil
repl:2:> (peg/match '(sequence (look 2) (capture 1)) "ab")
@["a"]

The website currently says something like:

(look offset patt)

Matches only if patt matches at a fixed offset. offset can be any integer. patt will not produce captures and the peg will not advance any characters.

If the current behavior is intentional, perhaps the docs could state the "optionalness" of patt. But then may be there is more to explain for the case where there is no patt specified as well.

I suppose changing the current behavior has a chance of breaking existing pegs.

Any thoughts?


For reference, source code currently has:

static void spec_look(Builder *b, int32_t argc, const Janet *argv) {
    peg_arity(b, argc, 1, 2);
    Reserve r = reserve(b, 3);
    int32_t rulearg = argc == 2 ? 1 : 0;
    int32_t offset = argc == 2 ? peg_getinteger(b, argv[0]) : 0;
    uint32_t subrule = peg_compile1(b, argv[rulearg]);
    emit_2(r, RULE_LOOK, (uint32_t) offset, subrule);
}
pepe commented 2 weeks ago

I consider it a feature, even if it is not documented. If I understand correctly, it checks if anything is at the offset. The code suggests this usage, too.

sogaiu commented 2 weeks ago

Yeah, I think it's safer (and possibly more convenient) to adjust the documentation.

pepe commented 2 weeks ago

The functionality with optional rules makes sense to me.

sogaiu commented 2 weeks ago

I'll close this for now and see if a pr to the docs will be accepted.