You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
These are just some thoughts I have when using it, they might not be correct, and only for reference. My English is not very good, so please excuse any offensive language because I might not realized. I will implement some features in my fork for preview, but I'm not proactive in writing tests.
remove state from pigeon. In my project's test, piegon is 15x slower than pointlander/peg(4.5s vs 0.3s), 7x because of i write and check a value in state(4.5 -> 2.3), and 3x because of empty state clone(2.3 -> 1.4), it's a terrible feature. Never enable it.
Consider about not return val by parseExpr, or replace by somthing. Just removed vals := make([]any, 0, len(seq.exprs)) from parseSeqExpr make my interpreter tests's time cost reduced from 1.4s to 0.85s. (My interpreter generate bytecode during parse, so i don't care what p.parse returned).
[implemented] Cancel the limitation of actionExpr(the most common way of writting golang code in peg file), we can write grammer like: expr <- firstPart:[0-9]+ { ... } secondPart:[a-z]+ {}
[implemented] Expose parser to actionExpr. The design of pigeon is assuming result of parse is AST and use peg grammer strictly. User should not care the parser, even can't access it. It's no need to hide at all.
[implemented] actionExpr need to return value, error, but in actual use, we hardly ever return an error. Meanwhile, when the parser is passed in, in a few necessary cases, we can directly call p.addError. Just return value and add return c.text at funcCallTemplate, it's no need to require a return expression.
Use stream (io reader)
[implemented] Remove exported API like ParseParseFileParseReader and so on. These apis return any to user, I don't think it's a good idea. Don't make them exported, let me decide what shoud be exported.
Change the design of Option , i think it's a side effect of hidding parser. it's over-abstraction.
[implemented] Provide a struct to embed, to replace the global state.
[implemented] Support string capture, it's a syntax from pointlander/peg: capture <- x:<'capture'> { fmt.Println(x) }
The text was updated successfully, but these errors were encountered:
Hi @fy0
Thanks for you suggestions (also in your other issues). At the moment, I do not have a plan nor the resources to work on a new major release. I mainly focus on the maintenance of the existing version. This is also the reason, why I am currently slow in replying to your issues.
Hi @fy0 Thanks for you suggestions (also in your other issues). At the moment, I do not have a plan nor the resources to work on a new major release. I mainly focus on the maintenance of the existing version. This is also the reason, why I am currently slow in replying to your issues.
I can understand. I like this project, because code is clear, it has a good structure and easy to customize.
But there's also problems exists for years, some of them are related to basic design and won't be fixed unless make breaking change(for example, the critical proformance problem). Therefore, it can't be a feature request, that why i named it 'new major suggestion' but it's not request for a new marjor verison.
And I agree with what you said, it's a difficult and requires a lot of resrouces to build a new marjor version.
I thought i wasn't the only one found these problems, I just recorded them, maybe it will helps.
These are just some thoughts I have when using it, they might not be correct, and only for reference. My English is not very good, so please excuse any offensive language because I might not realized. I will implement some features in my fork for preview, but I'm not proactive in writing tests.
remove state from pigeon. In my project's test, piegon is 15x slower than pointlander/peg(4.5s vs 0.3s), 7x because of i write and check a value in state(4.5 -> 2.3), and 3x because of empty state clone(2.3 -> 1.4), it's a terrible feature. Never enable it.
Consider about not return val by parseExpr, or replace by somthing. Just removed
vals := make([]any, 0, len(seq.exprs))
fromparseSeqExpr
make my interpreter tests's time cost reduced from 1.4s to 0.85s. (My interpreter generate bytecode during parse, so i don't care whatp.parse
returned).[implemented] Cancel the limitation of
actionExpr
(the most common way of writting golang code in peg file), we can write grammer like:expr <- firstPart:[0-9]+ { ... } secondPart:[a-z]+ {}
[implemented] Expose parser to
actionExpr
. The design of pigeon is assuming result of parse is AST and use peg grammer strictly. User should not care the parser, even can't access it. It's no need to hide at all.[implemented]
actionExpr
need to returnvalue, error
, but in actual use, we hardly ever return an error. Meanwhile, when the parser is passed in, in a few necessary cases, we can directly call p.addError. Just return value and addreturn c.text
at funcCallTemplate, it's no need to require a return expression.Use stream (io reader)
[implemented] Remove exported API like
Parse
ParseFile
ParseReader
and so on. These apis return any to user, I don't think it's a good idea. Don't make them exported, let me decide what shoud be exported.Change the design of
Option
, i think it's a side effect of hidding parser. it's over-abstraction.[implemented] Provide a struct to embed, to replace the global state.
[implemented] Support string capture, it's a syntax from pointlander/peg:
capture <- x:<'capture'> { fmt.Println(x) }
The text was updated successfully, but these errors were encountered: