Repository navigation
replace the all-in-one query parser by a complete one with AST/lexer/parser - #355
Conversation
MichaelMure
changed the title
cheshirekow
left a comment
cheshirekow
left a comment
There was a problem hiding this comment.
I just did a quick first pass and scanned through the change. In general I think breaking up the query logic into lexer/parser is a good idea. If I'm reading this right, a Filter is a collection of matchers for a list of fields. I have a slight concern that there will likely be a need to implement more complex logic. e.g. label is "foo" or (author is "me" and label is "bar") and so I'm not sure if it's worth implementing the framework for such a thing now. Probably that would mean implementing a filter as a tree of boolean evalutors.
Also, if the "field split" function is too coarse of a tool for the job, there's got to be a regex based lexer library already in go. If not, it is not too hard to implement a (slightly inefficient) one: store a list of (regex, token_type). Match the head of the string against every item in the list until you get a match. Pop the token off the head of the string. Repeat.
I'll try to find time to do a more comprehensive review. This coronavirus thing is really messing with my schedule.
Yeah I understand that. The current query language is inspired from the github one and is a compromise between expressiveness and shortness. As you can see in https://github.com/MichaelMure/git-bug/blob/query-parser-ast/cache/filter.go#L154-L184, the logic operator are implied, contextual and irregular: the one that should make the most sense is picked. Status, Author, Participant and Actor are OR combined, Label and Title are AND combined. I think it make sense to query a bug database and I hope it's enough to have an import filter (at least I don't see why it wouldn't). Maybe in the future explicit logical operator could be added but that would be a whole another level of complexity and I suspect not worth it at the moment. Maybe a solution would be to be able to specify multiple query to filter the imports ? It's a bit inelegant but could work. WDYT ?
The lexer is just the previous version refactored out. For some reason, writing a proper lexer has managed to escape my formation and my experience until now so I'll have to learn a bit before fixing that, unless someone want to play with that. At least it's now properly extracted and tested, ready for improvements.
Tell me about it :-| |
8fdb8ff to
314fcbb
Compare
|
Merging as this is an improvement. We can improve further later. |
|
More robust tokenizer in ecde909 |
2 participants
This PR introduce a new parser/compiler for the query DSL, with an intermediary representation in the form of an AST. By breaking down the previously single step into multiple stages (lexer/parser/compiler), it's now easier to test and improve, but more importantly, it's now possible to compile the AST into different target (bug matcher for the cache, query for a remote bugtracker in a bridge ...).
@cheshirekow it's a good first step towards #241. Could you have a look an see if you like it ?