Re: msi again

[Andrew Johnson <anj@aps.anl.gov>]

Hi Ben,

I agree with you on the semantics, with a couple of comments:

On Tuesday 24 August 2010 07:27:06 Benjamin Franksen wrote:
> We have two kinds of lists here that may be empty. By natural
> generalization of the above definition
>
> * an empty list-of-substitution-lists ("file xyz {}") means: do not insert
>   anything.

So does: file xyz { pattern {} }

Note that the pattern-names list there doesn't have to be empty, we should use 
the (absence of) pattern-definition braces to control whether we insert 
anything or not.

> * an empty list of substitutions means insert content of file with only
>   the global substitutions in scope (if any), thus "file xyz { {} {} }"
>   would insert the contents of file xyz twice.

Equivalent: file xyz { pattern {} {} }

I struggled to understand when anyone would want to use the "insert nothing" 
meaning, but since "file xyz {} {}" would not be legal to insert the same file 
twice, the distinction you make is sensible as it allows that.

Ok, on to the EBNF.  I have to be able to convert this grammar into Yacc for 
the dbLoadTemplate.y implementation, so I've rewriten some of your suggestions 
slightly, although the overall meaning should be the same.

> > substitution-file ::= ( global-definitions* template-substitutions+ )+
>
> This looks unnecessarily restrictive to me. Why not simply
>
>   substitution-file ::= ( global-definitions | template-substitutions )*

Ok.  My version required the file to end with a template-substitution, but if 
we're being liberal about it I guess that doesn't matter.

> > global-definitions ::= 'global' '{' variable-definitions '}'
> > variable-definitions ::= variable-definition ( ',' variable-definition )*
>
> The comma between definitions is not always required currently (in msi).

dbLoadTemplate also makes the comma optional at present and we do need to keep 
that behaviour, so agreed.

> I also (see above) want empty definition-lists, so my (counter-)proposal
> here is:
>
>   variable-definitions ::= ( variable-definition ','? )*

I accept the ability to write {} for empty lists, but when converting the EBNF 
into Yacc syntax it's much easier to define rules that are never empty, making 
their use conditional in the parent, thus:

global-definitions ::= 'global' '{' variable-definitions? '}'
variable-definitions ::= ( variable-definition ','? )+

> > template-substitutions ::= 'file' file-name '{' substitutions? '}'
>
> I'd rather push down the empty case into the variable-substitutions part
> (because the latter have a natural empty case, whereas a pattern-definition
> must start with a pattern-names clause). Thus
>
>   template-substitutions ::= 'file' file-name '{' substitutions '}'

Giving Yacc a rule that can match nothing generates shift/reduce conflict 
warnings, which I hate seeing.  I would rather have the EBNF more closely 
match the Yacc input, it makes development easier.  I did introduce another 
rule BTW which the Yacc code uses to extract the filename:

template-substitutions ::= template-filename '{' substitutions? '}'
template-filename ::= 'file' file-name

> > substitutions ::= variable-substitutions | pattern-substitutions
> > variable-substitutions ::= '{' variable-definitions '}'
>
> This should be
>
>   variable-substitutions ::= ( '{' variable-definitions '}' )*

Empty rule again, I have:

variable-substitutions ::= ( '{' variable-definitions? '}' )+


> > pattern-substitutions ::=
> >     'pattern' '{' pattern-names '}' pattern-definitions

I have a question about the above rule: In Yacc I can easily express the fact 
that you can't provide substitution values with no names for them thusly:

pattern_substitutions: PATTERN O_BRACE C_BRACE
    | PATTERN O_BRACE pattern_names C_BRACE
    | PATTERN O_BRACE pattern_names C_BRACE pattern_definitions
    ;

The following EBNF doesn't convey the same meaning though, it allows patterns 
that the Yacc would reject:

pattern-substitutions ::=
    'pattern' '{' pattern-names? '}' pattern-definitions?

The closest I can come up with is this, but it doesn't map onto the Yacc code 
terribly well since it takes 2 rules to Yacc's one:

pattern-substitutions ::= 'pattern' ( '{' '}' | pattern-block )
pattern-block ::= '{' pattern-names '}' pattern-definitions?

Do you have any better ideas?

>   pattern-names ::= ( variable-name ','? )*

Empty rule:

pattern-names ::= ( variable-name ','? )+


>   pattern-definitions ::= ( '{' pattern-values '}' )*

pattern-definitions ::= ( '{' pattern-values? '}' )+


>   pattern-values ::= ( value ','? )*

pattern-values ::= ( value ','? )+

> I'd like to agree with this proposal, as we already broke compatibility
> when introducing scopes, and I never understood the reason for being so
> liberal with variable names.

I have an explanation for that; it reduces the amount of code you have to 
write in the lexer if you only have one definition for what Perl calls a 
Bareword, i.e. something that you're really capturing as a string but you 
don't want users to have to put quotes round.  dbLoadTemplate_lex.l has a 
single token WORD it uses for all Bareword elements, and actually I'm not 
quite sure how to tell it to look for a different kind of Bareword; normally 
information flows from Lex to Yacc but not backwards (it probably involves 
writing start states, which I haven't used before myself).

> In fact, I would further limit them to
>
>   variable-name ::= variable-name-start-char variable-name-char*
>   variable-name-start-char ::= [a-zA-Z]
>   variable-name-char ::= [a-zA-Z0-9_]

I would want to add _ to variable-name-start-char, but otherwise I agree with 
those.  However...

> like in most programming languages (LISP being the only exception I am
> aware of). On the other hand, adapting substitution files to the new
> scoping rules is trivial (if tedious, just add the missing definitions),
> but adapting variable names could be quite disruptive; we might want to be
> careful here.

I think we have to leave the original name definitions in until 3.15 at least.

> For reference, I have attached the complete grammar with the changes I
> proposed.

Returning mine, which I have working in dbLoadTemplate.y.  The following a 
working replacement for the regular makeBaseApp example template BTW:

# Example substitutions file
global { user = anjHost }

file "db/dbExample1.db" { {} }

global {
    scan = "$(no) second"
}
file db/dbExample2.db {
    pattern { no, scan }
        { 1 }
        { 2 }
        { 3, "5 second" }
}

- Andrew
-- 
The best FOSS code is written to be read by other humans -- Harald Welte

EBNF grammar for the template substitutions file format, to be understood
by msi and dbLoadTemplate.  These rules have been written to be easily
converted into a Yacc grammer, so they don't usually match nothing.

substitution-file ::= ( global-definitions | template-substitutions )*

global-definitions ::= 'global' '{' variable-definitions? '}'

template-substitutions ::= template-filename '{' substitutions? '}'
template-filename ::= 'file' file-name
substitutions ::= pattern-substitutions | variable-substitutions

pattern-substitutions ::= 'pattern' ( '{' '}' | pattern-block )
pattern-block ::= '{' pattern-names '}' pattern-definitions?

pattern-names ::= ( variable-name ','? )+
pattern-definitions ::= ( '{' pattern-values? '}' )+
pattern-values ::= ( value ','? )+

variable-substitutions ::= ( '{' variable-definitions? '}' )+
variable-definitions ::= ( variable-definition ','? )+
variable-definition ::= variable-name '=' value


These definitions are use by the lexer:

variable-name ::= variable-name-start-char variable-name-char*
file-name ::= file-name-char+ | double-quoted-string | single-quoted-string
value ::= value-char+ | double-quoted-string | single-quoted-string

double-quoted-string ::= '"' (double-quoted-char | escaped-char)* '"'
single-quoted-string ::= "'" (single-quoted-char | escaped-char)* "'"
double-quoted-char ::= [^"\]
single-quoted-char ::= [^'\]
escaped-char ::= '\' .


In R3.15 we'd like to restrict the bareword characters to these:

variable-name-start-char ::= [a-zA-Z_]
variable-name-char ::= [a-zA-Z0-9_]
file-name-char ::= [a-zA-Z0-9_+:./\] | '-'
value-char ::= [a-zA-Z0-9_+:./\<>;[] | '-' | ']'

but for R3.14.12 we'll stay with this for all of those:

*-char ::= [a-zA-Z0-9_+:./\<>;[] | '-' | ']'