Re: [Libguestfs] [PATCH 2/3] v2v: add Var_expander

On Monday, 25 March 2019 15:55:46 CET Richard W.M. Jones wrote: > On Mon, Feb 25, 2019 at 05:22:51PM +0100, Pino Toscano wrote: > [...] > > After being burned a few times with custom parsing (hello, guestfish) > I'm not a big fan. I can perfectly understand that, however ... > Is there not an existing C or OCaml library/facility we could use > here? It's a shame we can't use Perl Template Toolkit because it > would be ideal here. ... sadly I did not find anything simple convering the use case that this Var_expander module covers. The closest thing I found was the usage of ${..}/$(..) variables in dune (the OCaml build system), with the following differences: - manually tokenizes the string - allows anything as variable name, splitting it in two if a ':' is found - ${..}/$(..) instead of %{..} > There are all kinds of questions that aren't answered such as: Should > variables be replaced recursively? > [...] > Should we allow loops or similar constructs? I don't think they are needed. > How do you escape %{..} if you don't want it to be replaced? Good question... dune does not seem to allow that; maybe we can allow a trailing '$'/'\' as escape character.

> Existing template systems solve these kinds of problems already. Yes, however they are much more complex, and with a number of features like conditionals, filters, loops, etc. Considering this is needed so far for the file paths of disks in -o json, IMHO plain variables are enough. > > +let var_re = PCRE.compile "%{([^}]+)}" > > Are we planning to allow a completely free choice for variable names, > or could we limit this regexp to only matching ASCII alphanumeric + > underscore? This regex allows anything as variables for two reasons: 1) we do not miss any variable-like pattern (so we do not silently skip any now, while handling it in the future in case we accept more characters)

> > +let check_variable var = > > + String.iter ( > > + function > > + | '0'..'9' > > + | 'a'..'z' > > + | 'A'..'Z' > > + | '_' > > + | '-' -> () > > + | _ -> raise (Invalid_variable var) > > + ) var > > ... and then this function would presumably go away. 2) we can check that a variable has only allowed characters, and report that to the user