[Rpm-maint] [rpm-software-management/rpm] implement %elif (#613)

[pavlinamv]

pavlinamv commented on this pull request.



>  	spec->readStack = rl;
 	spec->line[0] = '\0';
+    } else if (isElif) {
+	spec->readStack->reading = match && spec->readStack->readable;
+	if (spec->readStack->reading)
+	    spec->readStack->readable = 0;
+	spec->line[0] = '\0';
+	match = -1;

Thank you for the comment. 

> For example, isn't elifEnabled practically the same thing as checking 
> whether ->next non-NULL?

No. Because `elifEnabled = 1` if some `%if` condition starts and does not reach `%else` or `%endif`. `->next` is non-NULL if some `%if` starts and does not reach `%endif`. And from all other `->` variables you can not decide whether in the current `%if` `%else` was reached or not. Yes the meaning and name of the variable can be different, but there must be some variable containing this information.

> Also makes me wonder if it'd fit in more naturally if you just treated 
> %elif as a new %if inside an %else, which is what it ultimately is. 
> %elif is just syntactic sugar after all.

This alternative approach does not optimize variables, or complexity of the program:  
Programming `%elif` like a "special `%else` `%if` " will lead into:
- `->elifEnabled` must stay in the code with this approach too. (Of course no metter how it is implemented, there is need for testing whether all %elif's are on their propper places). 
- even if in this approach `->reading` can be omitted, there must be some new 
variable counting how nested the `%elif` is. It is because after `%else` resp. `%endif`, function `readLine`, must close the main `%if`, and all other "auxiliary %else %if's" that emulate `%elifs`.
Thus it will result into code that is not better than the current one.

Current approach corresponds to defining %elif straightforwardly according to its syntax and the alternative approach corresponds to defining %elif using %if inside an %else. Both are correct definitions, but for me the current looks more readable and natural.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/613#discussion_r246720538
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rpm.org/pipermail/rpm-maint/attachments/20190110/2a71aad6/attachment.html>