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

>  * The "repeat" command may cause crashes or leaks.

Weren't we thinking of removing the repeat command at one point?


Reply via email to