> the two cases only differ by the actual token, you should handle them 
> by the same code and without unnecessary + 5 "magic" calculations.

Will be improved in the next version.

> auxBuf is not used to modify the contents so it should be const

Will be changed in the next version.

> Having several places parse the same tokens is generally an indication 
> of something being wrong or at least sub-optimal, why can't this be 
> handled where %else and %endif are otherwise parsed? (I'd think macro 
> expansion shouldn't matter for this)

It is because logically it should be done before the macro expansion. 
In majority of cases macro expansion shouldn't matter. But there are corner 
cases like:
   %else %{?iii:this}
   %else %define name value
such that the macro expansion matter for them. I prefer to solve all possible 
cases to handle the warnings where %else and %endif are otherwise parsed.
Moreover in the future there may be another cases, where macro expansion should 
matter in more cases e.g. %elif.


> "Not allowed" as an expression doesn't work this kind of context: it's 
> not question whether you have a permission to place text someplace or 
> not, it's simply a syntax error.

I will change the error message in next version.


> Text following %endif is obviously a syntax error, but in case of %else 
> I think it'd be more useful to simply treat any following text as a part 
> of the else-branch. Ie, make "%else %if 0%{?centos}" kind of thing 
> actually work.

There are 4 types of texts after %else that I saw: 
    a) %else # 0%{?with_systemd}
    b) %else %if %{with_prerelease}
    c) %else if 0%{?rhel} >= 6
    d) %else !use_embedded
Treating a text after %else as a part of the else-branch will improve a) and 
b), but it will not solve case c) and especially d). That is why I would prefer 
the warning in both cases.

-- 
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/625#issuecomment-460289113
_______________________________________________
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Reply via email to