Re: [Rpm-maint] [rpm-software-management/rpm] Add ternary operator support to expression parser (#838)
Merged #838 into master. -- 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/838#event-2643253274___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add ternary operator support to expression parser (#838)
Okay, looks nice and contained to me. Thanks a bunch! -- 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/838#issuecomment-532654226___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add ternary operator support to expression parser (#838)
mlschroe commented on this pull request.
> + result = v1->data.i != 0;
+ break;
+ case VALUE_TYPE_STRING:
+ result = v1->data.s[0] != '\0';
+ break;
+ default:
+ goto err;
+}
+valueFree(v1);
+if (rdToken(state))
+ goto err;
+v1 = doTernary(state);
+if (v1 == NULL)
+ goto err;
+if (state->nextToken != TOK_TERNARY_ALT) {
+ exprErr(, _("syntax error in expression"), state->p);
Damn. I'm copying from the wrong line ;)
--
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/838#discussion_r325630357___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add ternary operator support to expression parser (#838)
pmatilai commented on this pull request.
> + result = v1->data.i != 0;
+ break;
+ case VALUE_TYPE_STRING:
+ result = v1->data.s[0] != '\0';
+ break;
+ default:
+ goto err;
+}
+valueFree(v1);
+if (rdToken(state))
+ goto err;
+v1 = doTernary(state);
+if (v1 == NULL)
+ goto err;
+if (state->nextToken != TOK_TERNARY_ALT) {
+ exprErr(, _("syntax error in expression"), state->p);
expression.c: In function ‘doTernary’:
expression.c:712:15: warning: passing argument 1 of ‘exprErr’ from incompatible
pointer type [-Wincompatible-pointer-types]
712 | exprErr(, _("syntax error in expression"), state->p);
| ^~
--
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/838#pullrequestreview-289857702___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add ternary operator support to expression parser (#838)
@mlschroe: Thank you. The PR looks good to me. @pmatilai After the mentioned example there is a list of operators that can be used in a condition after ``` %if ``` -- 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/838#issuecomment-532644399___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add ternary operator support to expression parser (#838)
I added the ternary operator to the list of supported operators. -- 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/838#issuecomment-532641740___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add ternary operator support to expression parser (#838)
@pavlinamv , what do you mean by changing those conditionals in the documentation? I don't see them needing changing because of this, but maybe I'm missing something. -- 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/838#issuecomment-532641217___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add ternary operator support to expression parser (#838)
Could you change file
```
rpm/doc/manual/spec
```
in section "Conditionals" the text after example:
```
%if 0%{?fedora} > 10 || 0%{?rhel} > 7
```
according of the PR?
--
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/838#issuecomment-532630700___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add ternary operator support to expression parser (#838)
Thank you. It solves the problem. -- 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/838#issuecomment-532561398___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add ternary operator support to expression parser (#838)
I've opened another pull request that fixes the error printing as this is unrelated to the ternary operator. -- 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/838#issuecomment-532217959___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add ternary operator support to expression parser (#838)
I modeled the code after the other operators. Things like `--eval '%{expr: 4
+}'` have the same problem.
--
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/838#issuecomment-532193835___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add ternary operator support to expression parser (#838)
I think that adding ternary operator support to the expression parser is useful
and it will not cause problems in the parsing of the existing expressions.
There is one thing that can be improved -cases like:
```
--eval '%{expr: 1 ?}'
--eval '%{expr: 0 ? 3 : }'
```
returns an error, but without any message. It can be improved.
--
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/838#issuecomment-532151451___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
[Rpm-maint] [rpm-software-management/rpm] Add ternary operator support to expression parser (#838)
You can view, comment on, or merge this pull request online at:
https://github.com/rpm-software-management/rpm/pull/838
-- Commit Summary --
* Do not expand %{expr:} again after evaluating the expression
* Catch unterminated strings in expression parsing
* Support ternary operator in expression parser
* Add testcase for the ternary operator
-- File Changes --
M rpmio/expression.c (71)
M rpmio/macro.c (10)
M tests/rpmmacro.at (30)
-- Patch Links --
https://github.com/rpm-software-management/rpm/pull/838.patch
https://github.com/rpm-software-management/rpm/pull/838.diff
--
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/838
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
