This whole thing is a seriously powerful and cool thing. Million thanks for all
the work, @mlschroe !
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
Awesome, thanks!
--
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/846#issuecomment-534030947___
Rpm-maint mailing list
Merged #846 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/846#event-2653832413___
Rpm-maint mailing list
Changed and force-pushed. I also tweaked the code so that a negative number is
allowed to make `%[ 3 + %[ 4 - 5]]` work.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
Yup, that'd make it much easier to understand what it's complaining about.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
Ah, I get what you mean. I think we should print a bare word error if the
expanded string does not start with a digit. This makes it similar to what
`%{expr:...}` and `%if` does:
```
$ ./rpm --define "aaa a" --define "bbb 123b" --eval '%{expr: %aaa }'
error: bare words are no longer supported,
Not sure that message makes it any clearer, I probably failed to explain why I
find it confusing to begin with. I guess the problem is that it doesn't explain
*why* it expects an integer there, and that makes it sound like it will *only*
accept a number there, which in a macro context seems
Sorry for the multiple force pushes, I had a little fight with git.
I now use "macro expansion did not return a number" as error message.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
@mlschroe pushed 1 commit.
52bd4a99ca452a974b248cf9291454e992e263b3 Implement short-circuit for logical
and ternary operators
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
Yeah, looks that little bit nicer that way, thanks. I don't particularly love
camelCase but when everything in the surroundings uses it...
There's an unrelated indentation change for the division-by-zero case, and
trailing whitespace after the doExpressionExpansion() function. I could live
I switched the helper function to camelCase and moved the digit check into a
separate function to make the code easier to read.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
@mlschroe pushed 2 commits.
713d13cb402a76f58be3513fdd8e197754390265 Add support for primary expansion to
the expression parser
11e11cf1fc187efc50e4cdff1036355032f97be3 Implement short-circuit for logical
and ternary operators
--
You are receiving this because you are subscribed to this
I also noticed it, but didn't want to check if it's still true ;)
I removed that sentence.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
Thanks. I'll need to take a proper look at this all, but this caught my eye
from the docs as it's getting moved around in the patch:
> There is currently an 8K limit on the size that this macro can expand to.
That's a *little* outdated info, might want to drop it while at it :D
--
You are
I now also documented the difference to `%{expr:}`
--
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/846#issuecomment-533100283___
@mlschroe pushed 2 commits.
c2469258af1915aeef5b44a6c1444a7a938d99d0 Add support for primary expansion to
the expression parser
d77df7f2275027228f15b674bbab59d9496adb61 Implement short-circuit for logical
and ternary operators
--
You are receiving this because you are subscribed to this
I fixed the comment plus added some basic documentation.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
@mlschroe pushed 2 commits.
02819e9b723f29329acaed20051f5a0cee10121a Add support for primary expansion to
the expression parser
3288db0c688e89131833258d9cac028fabf1e89e Implement short-circuit for logical
and ternary operators
--
You are receiving this because you are subscribed to this
pavlinamv commented on this pull request.
> @@ -1349,6 +1386,13 @@ expandMacro(MacroBuf mb, const char *src, size_t slen)
doShellEscape(mb, s, (se - 1 - s));
s = se;
continue;
+ case '[': /* %[...] expression expansion */
+ if
Yes, I just ran out of time yesterday but wanted to show your guys the current
state.
I've added some test cases, next comes the documentation.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
Yup, some testcases (along with docs) that show the behavior differences to
%{expr:...} would be appreciated.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
Cool. It would be great to add a testcase.
--
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/846#issuecomment-533026052___
@mlschroe pushed 1 commit.
630966ce35c39ac5a8c2a2583099a4165ac15c6f Implement short-circuit for logical
and ternary operators
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
This adds %[ expr ] as a new means to do expression expansion in rpm. Unlike
%{expr:} the expression is expanded in the parser, so we are safe against
expansion results messing up the syntax and also can to short-circuiting.
You can view, comment on, or merge this pull request online at:
24 matches
Mail list logo
