RieksJ commented 9 years ago

For purposes such as:

(better) support testing (e.g. to conditionally include/select test populations),
facilitate the compilation of different prototypes and/or fspecs in a project that has multiple ADL files, where each prototype uses a different subset of the project's ADL files,
allow others to play with an adl file in a stand-alone fashion so as to evaluate its use or needed changes for a project (e.g. by conditionally including a set of demo-interfaces and demo-populations - an example of which would be an ADL file that allows users to login and do some account management).

parser directives are called for. Realizing that this increases the "beheerslast", parser directive syntax) should be ADL-agnostic. For starters (i.e. to accommodate purposes 1, 2 and 3), the following (minimum) syntax is proposed for an <if-expression>:

 #IF <condition> 
 <textlines>
 #ENDIF

and <set-line> and <clr-line>

 #SET <variablename> (TRUE | FALSE)
 #CLR <variablename>

This can be specified in a BNF-like notation as follows:

<if-expression> ::= <if-line> <textlines> <endif-line>
<if-line> ::=  #IF <condition> 
<condition> ::=  [NOT] <variablename> [EXISTS]
<textlines> ::= ( <ADL text>  |  <set-line> | <clr-line> )
<set-line> ::= #SET <variablename> (TRUE | FALSE)
<clr-line> ::= #CLR <variablename>
<endif-line> ::= #ENDIF

stefjoosten commented 9 years ago

I don't like this idea. Let us keep the issue here as a place to collect insights on this issue. For now, we will leave this issue unhandled without closing it. (Is there a kind of hibernate mode for this type of issue?)

RieksJ commented 9 years ago

@stefjoosten What in particular is it that you do not like? Any of its purposes perhaps?

hanjoosten commented 9 years ago

I don't like the idea of Rieks too. It is already possible to split definitions into separate files, using INCLUDE. Doing this in a smart way, using comments for whoever is going to use any specific configuration, should enable most of the required possibilities.

Also, looking at the proposed syntax, this uses variables that can be set and cleared. Scary!

My main objection is the lack of balance between usage and 'beheerslast'.

My 2c...

RieksJ commented 9 years ago

@stefjoosten What in particular is it that you do not like? Any of its purposes perhaps?

Now that there is a new parser, is it still really that cumbersome?

RieksJ commented 7 years ago

I still have not heard any other argument other than that the beheersrlast is too big, without any specification stating what the beheerslast constitutes of. I find that quite frustrating as I have written preparsers in Forth, which have not provided any beheerslast other than defining/creating it, and it was surprisingly simple.

Note that I'm talking here about ADL-agnostic and preparsers; for that, in particular, I cannot fathom what the beheerslast would be.

Also note that I'm not talking here about say macro's for implementing equivalence relations (which I think is a good idea as well, and I can see where beheerslast would come there)

If, for example, you look at the SIAM module right now, the ever increasing number of small files and the beheerslast for maintaining them such that you can actually do some configuration in the SIAM-Module-include file that you have to make is quite big, too. But then, if you don't use it, you don't have the problems.

RieksJ commented 7 years ago

The work that I've been doing since I originally created this ticket has convinced me of the fact that

INTERFACEs cannot be reused in the same way as we can re-use concepts, relations and rules (using INCLUDEs), and
there is a real benefit to be able to do so.

In my perception, this tips the balance between usage and 'beheerslast' (as mentioned by @hanjoosten) of implementing this pre-processing feature in favor of the users/programmers.

The following snippet exemplifies the user/programmer beheerslast that could be avoided (the example is taken from a real project):

         , "Details"   : I cRud COLS
            [ "Control for": isCtrlOfObj cRUd 
            , "Description": objDescr cRUd 
#IF ToDoListModule EXISTS
            , "To do list": objToDoMsg cRud
#ENDIF 
            ]
#IF SourceDocReferenceModule EXISTS
         , "Source doc": objSrcID;srcID~ cRud BOX <ROWSNL>
            [ "Show reference info": I-srcEditRefInfoFlg cRud COLS
               [ "Source document reference info": srcRef cRud
               , "URL of source doc": srcURL cRud
               , " ": I cRud BOX <ROWSNL> [ "Edit Reference": I cRud BOX <PropertyButton> [ property : srcEditRefInfoFlg cRUd ] ]
               ]
            , "Edit reference info": srcEditRefInfoFlg cRud COLS   
               [ "Source document reference info": srcRef cRUd
               , "URL of source doc": srcURL cRUd
               , " ": I cRud BOX <ROWSNL> [ "Done editing": I cRud BOX <PropertyButton> [ property : srcEditRefInfoFlg cRUd ] ]
               ]
#ENDIF
            ]

This snippet can compile into 4 variants, depending on the settings of the variables 'ToDoListModule' and 'SourceDocReferenceModule'. Currently, programmers would need to create an interface for every of this variant, and INCLUDE the right interface whenever needed. Also, any change in one of these interfaces would have to be propagated to the other interfaces. Thus, having the pre-processing features reduces the user/programmer beheerslast of this interface by approximately a factor 4.

Not having the pre-processing feature does not incentivise module-makers to create reusable interfaces. An example is the SIAM module, that could provide quite generic and nice interfaces e.g. for logins, accountmanagement and such, that would adapt to the SIAM features that would be included or excluded. Currently though, SIAM module users make such interfaces themselves.

Having a pre-processing feature could turn the tables: whenever a project comes up with a very nice interface (for SIAM, that would be MPTrx, which has a nice interface with buttons, registration, etc.), it can be readily converted into a more generically useful interface and it could be included in the SIAM module for re-use by others.

hanjoosten commented 6 years ago

Since this is an issue no-one will put effort into, I will close this.

RieksJ commented 6 years ago

Hidde-Jan is going to do a quick impact-analysis.

In order to follow through on a strict separation between ADL and the preparser, the syntax as described in the first entry of this issue will need to be extended with

#INCLUDE <fileid>

where

<fileid> is a string that contains a path+filename and semantics that are the same as what the INCLUDE-statement has.

bartMarinissen commented 6 years ago

Hi,

At the request of Rieks, I am working on something along these lines. More to come later.

hanjoosten commented 6 years ago

Just my 2p:

In order to keep this stuff out of the parser itself, the parser mustn't be modified.
I would suggest that the syntax of all directives are comments in adl. Then these files remain valid adl.
The feedback in the error messages of ampersand contain the line and column number of the symbols. It would become hard for the user if these numbers become wrong.

hanjoosten commented 6 years ago

@bartMarinissen : don't hesitate to contact me if you want help in any way. We could have a skype session where I can tell you about the current code.

bartMarinissen commented 6 years ago

@hanjoosten sounds good. Shoot me an e-mail at bart.marinissen@tno.nl and we can set something up.

bartMarinissen commented 6 years ago

I am currently working on this at the following branch: https://github.com/bartMarinissen/Ampersand/tree/feature/pre-compiler. Almost all of the work for hooking into the parser is done in commit: https://github.com/bartMarinissen/Ampersand/commit/0015e7363b712b46145429f27a6c145d0cc03006

The remainder of the work is to actually implement a pre-processor. The current version has something that seems to work, but has ugly code for the moment.

This implimentation takes a slightly different approach than suggested. Rather than using

#SET <variablename> (TRUE | FALSE)
#CLR <variablename>

to set variables, they are set in INCLUDE statements. The current syntax for that is INCLUDE "filename" ["List", "Of", "Defined", "Variables"] I think that should become something closer to INCLUDE "filename" -- [List Of Defined Variables] That is, put the variables in a comment, and have them simply be whitespace-delimited.

Moreover, the current implementation does not have negation in the IF statement (or an ELSE clause) This severely limits the expressive power, so the 'guard' of the IF statement should at least include some way to negate. Perhaps we can also easily add simple predicate logic.

RieksJ commented 6 years ago

Ik wil graag even met je sparren om na te gaan of dit in onze behoeften voorziet. Ik stel me (bijvoorbeeld) voor dat modules geladen kunnen worden door het INCLUDEn van 1 bestand. Volgens het voorstel moet je daar dan de variabelen aan meegeven die daarin nodig zijn. Echter, dat ene bestand doet zelf ook weer INCLUDEs, en de daar ge-include bestanden hebben nu juist die variabelen nodig. Misschien moeten we (samen met Michiel) een paar test-cases specificeren.

Rieks

From: Bart Marinissen notifications@github.com Sent: vrijdag 7 september 2018 17:00 To: AmpersandTarski/Ampersand Ampersand@noreply.github.com Cc: Joosten, H.J.M. (Rieks) rieks.joosten@tno.nl; State change state_change@noreply.github.com Subject: Re: [AmpersandTarski/Ampersand] ADL-agnostic (pre)Parser Directives (#33)

I am currently working on this at the following branch: https://github.com/bartMarinissen/Ampersand/tree/feature/pre-compiler. Almost all of the work for hooking into the parser is done in commit: bartMarinissen@0015e73https://github.com/bartMarinissen/Ampersand/commit/0015e7363b712b46145429f27a6c145d0cc03006

The remainder of the work is to actually implement a pre-processor. The current version has something that seems to work, but has ugly code for the moment.

This implimentation takes a slightly different approach than suggested. Rather than using

SET (TRUE | FALSE)

CLR

to set variables, they are set in INCLUDE statements. The current syntax for that is INCLUDE "filename" ["List", "Of", "Defined", "Variables"] I think that should become something closer to INCLUDE "filename" -- [List Of Defined Variables] That is, put the variables in a comment, and have them simply be whitespace-delimited.

Moreover, the current implementation does not have negation in the IF statement (or an ELSE clause) This severely limits the expressive power, so the 'guard' of the IF statement should at least include some way to negate. Perhaps we can also easily add simple predicate logic.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHubhttps://github.com/AmpersandTarski/Ampersand/issues/33#issuecomment-419467032, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AIILXSmuzg0wFegl5n3nqdyCaQxcOGd1ks5uYonogaJpZM4DmIjp. This message may contain information that is not intended for you. If you are not the addressee or if this message was sent to you by mistake, you are requested to inform the sender and delete the message. TNO accepts no liability for the content of this e-mail, for the manner in which you use it and for damage of any kind resulting from the risks inherent to the electronic transmission of messages.

bartMarinissen commented 6 years ago

Ik stel me (bijvoorbeeld) voor dat modules geladen kunnen worden door het INCLUDEn van 1 bestand. Volgens het voorstel moet je daar dan de variabelen aan meegeven die daarin nodig zijn. Echter, dat ene bestand doet zelf ook weer INCLUDEs, en de daar ge-include bestanden hebben nu juist die variabelen nodig.

Wat je schetst klopt, met een extra feit. Variabelen die 'aan' staan worden gepropageerd in includes dus:

file0.adl:
----------
INCLUDE file1.adl ["debug"]

file1.adl:
----------
INCLUDE file2.adl

file2.adl:
----------
-- IF debug
debugging code here
-- ENDIF

In dit geval zou de debugging code actief zien, omdat de "debug" flag doorgegeven wordt. Een include geeft alle 'flags' mee die aan staan in het bestand waar de include gedaan wordt.

Een aantal test-cases lijkt me nog steeds nuttig, maar ik hoop/verwacht dat dit nog steeds in de behoeften voorziet. Mocht ik het fout hebben, of als er nog iets anders is, als je een keer langsloopt kunnen we dit best bespreken.

P.S. When I said:

Een include geeft alle 'flags' mee die aan staan in het bestand waar de include gedaan wordt.

I realized this might be a problem if a file is included twice in different contexts. I am not sure how the current code would behave in that case.

Michiel-s commented 5 years ago

@RieksJ, @bartMarinissen, what is the status of the preparser? There is still an open PR #858. Can we close the issue already?

RieksJ commented 5 years ago

No, we cannot (yet). After I reviewed the stuff for the first time (and accepted it), I found a bug. This has to be fixed before merging it.

hanjoosten commented 5 years ago

Travis-ci reports this bug due to the testcase of @RieksJ

RieksJ commented 5 years ago

That's good, because that's the bug I found.

bartMarinissen commented 5 years ago

Christmas holiday got in the way, sorry for the delay. I expect that, after the last commit on feature/preProcessor, the tests will pass.

Michiel-s commented 5 years ago

PR #858 is merged

AmpersandTarski / Ampersand

ADL-agnostic (pre)Parser Directives #33

SET (TRUE | FALSE)

CLR