Closed gilbert closed 1 year ago
norskeld
commented
1 year ago Hi! Thanks for the issue and the diff, much appreciated. Kinda ashamed I've overlooked this. I'll roll out a fix on the weekend.
I can't figure out how to get past the pre-commit hooks
That's a bummer, can you share the specifics? Maybe I could help.
gilbert
commented
1 year ago No problem, and thank you for creating this beauty of a library ☺️
Here's the output when I try to commit:
norskeld
commented
1 year ago Here's the output when I try to commit:
git commit output
Okay, I figured it out. tl;dr: should have done cd docs && npm i before committing.
We have an eslint plugin for imports, and right now it's configured like this:
"import/resolver": {
"typescript": {
"alwaysTryTypes": true,
"project": ["./tsconfig.json", "./*/tsconfig.json"]
}
}The culprit here is "./*/tsconfig.json", which instructs the plugin to pick up not only the root tsconfig, but also the one in docs which in turn extends @vue/tsconfig/tsconfig.web.json, so, since docs didn't have its dependencies installed, everything failed.
The proper way to address this and some other issues with the current setup would be to migrate to monorepo setup, but I have no capacity to do that right now. So for the time being we will install deps for all packages in postinstall when doing npm install (done in #78).
Describe the bug
When
sepByfails to match at all, it still updates the cursor position. This causes subsequent parsers to fail due to skipped input.In contrast,
many– which also always succeeds – does not update cursor position when it fails to match at all.Reproduction
I can't figure out how to get past the pre-commit hooks, so here's a diff of the test and fix instead of a PR:
The test & fix
```diff diff --git a/src/__tests__/combinators/sepBy.spec.ts b/src/__tests__/combinators/sepBy.spec.ts index ca348b9..0cda6a1 100644 --- a/src/__tests__/combinators/sepBy.spec.ts +++ b/src/__tests__/combinators/sepBy.spec.ts @@ -1,4 +1,4 @@ -import { sepBy, sepBy1 } from '@combinators' +import { sepBy, sepBy1, sequence } from '@combinators' import { string } from '@parsers' import { run, result, should, describe, it } from '@testing' @@ -26,6 +26,17 @@ describe('sepBy', () => { should.matchState(actual, expected) }) + + it('should successfully continue if nothing matched', () => { + const parser = sequence( + sepBy(string('hello'), string('?')), + sepBy(string('bye'), string('?')), + ) + const actual = run(parser, 'bye?bye?') + const expected = result(true, [[], ['bye', 'bye']]) + + should.matchState(actual, expected) + }) }) describe('sepBy1', () => { diff --git a/src/combinators/sepBy.ts b/src/combinators/sepBy.ts index 51b81b1..f42bc32 100644 --- a/src/combinators/sepBy.ts +++ b/src/combinators/sepBy.ts @@ -14,6 +14,7 @@ import type { Parser } from '@types' export function sepBy): Parser> {
return {
parse(input, pos) {
// Run the parser once to get the first value.
const resultP = parser.parse(input, pos)
@@ -37,8 +38,8 @@ export function sepBy(parser: Parser, sep: Parser ): Parser>
return {
isOk: true,
- span: [pos, resultP.pos],
- pos: resultP.pos,
+ span: [pos, pos],
+ pos,
value: []
}
}
```
And here's the test again for visibility, which fails on the current version (3.6.2):