On Wed, Nov 17, 2021 at 08:40:09PM +0100, Dominik Vogt wrote: > 'k, the patched code didn't immediately crash, so here it is (two > patches). Please test.
I've applied those two patches on a branch called `new-parser`. So far, I've tested this on approximately five different configuration files and haven't noticed anything unusual or anything which breaks. Which is a good sign. > The new code: > > * Rewrites functions.c to use hooks that are provided by a > pluggable parser module (even at run time). > * The module that replicates the old parser behaviour is in > cmdparser_old.[ch]. > * The long term goal is to replace the _old parser with a new one > that evauates the BNF definitions and does the parsing of > function arguments before actually calling them. > * To allow that, a "parsing context" structure is passed to the > CMD_... functions. This is already implemented but not used. > * How the "parsing context" structure should look like is yet to > be defined. > * It shoul be entirely possible to convert command functions one > by one to the new type of parser. So that word does not need > to be done in a single big step. This is sensible, and from a quick glance at what's there at the moment, it makes sense. I'm hoping that we can also derive the execute_context_t from the parsed command as well, which would possibly help in unifying some of the command syntax in the future. I did wonder whether we might want to consider yacc/bison for the grammar of the commands, but I've never personally been a fan of it, but it could help wih some of the raw parsing... > Possible pitfalls: > * Bad behaviour of the fvwm_debug function because of wrong > parameters being passed. I did a quick santity check of the ones which were pulled in from your changes, and they look fine. All other parser debug is going to stderr anyway. > * The "repeat" command may cause crashes or leaks. Weren't we thinking of removing the repeat command at one point? Kindly, Thomas
