1 |
Dnia 15 maja 2016 15:31:29 CEST, Jan Chren <dev.rindeal@×××××.com> napisał(a): |
2 |
>- fix case: |
3 |
> - `CFLAGS='-O1 -O2'` |
4 |
> - `get-flag '-O*'` |
5 |
> - before `-O1` |
6 |
> - now `-O2` |
7 |
>- fix case: |
8 |
> - `CFLAGS='-W1,-O1'` |
9 |
> - `get-flag '-O*'` |
10 |
> - before `-W1,O1` |
11 |
> - now return 1 |
12 |
> |
13 |
>`get-flag march` == "i686" syntax still works. |
14 |
|
15 |
Could you add appropriate test cases, in the tests subdirectory? |
16 |
|
17 |
>--- |
18 |
> eclass/flag-o-matic.eclass | 13 +++++++++---- |
19 |
> 1 file changed, 9 insertions(+), 4 deletions(-) |
20 |
> |
21 |
>diff --git a/eclass/flag-o-matic.eclass b/eclass/flag-o-matic.eclass |
22 |
>index e0b19e9..f670320 100644 |
23 |
>--- a/eclass/flag-o-matic.eclass |
24 |
>+++ b/eclass/flag-o-matic.eclass |
25 |
>@@ -535,7 +535,7 @@ strip-unsupported-flags() { |
26 |
> # @DESCRIPTION: |
27 |
> # Find and echo the value for a particular flag. Accepts shell globs. |
28 |
> get-flag() { |
29 |
>- local f var findflag="$1" |
30 |
>+ local var findflag="${1}" |
31 |
> |
32 |
> # this code looks a little flaky but seems to work for |
33 |
> # everything we want ... |
34 |
>@@ -543,11 +543,16 @@ get-flag() { |
35 |
> # `get-flag -march` == "-march=i686" |
36 |
> # `get-flag march` == "i686" |
37 |
> for var in $(all-flag-vars) ; do |
38 |
>- for f in ${!var} ; do |
39 |
>- if [ "${f/${findflag}}" != "${f}" ] ; then |
40 |
>- printf "%s\n" "${f/-${findflag}=}" |
41 |
>+ # reverse loop |
42 |
>+ set -- ${!var} |
43 |
>+ local i=$# |
44 |
|
45 |
You are using $ with and without braces inconsistently. Please stick to one form. |
46 |
|
47 |
>+ while [ $i -gt 0 ] ; do |
48 |
|
49 |
Please use [[ ]] for conditionals. It has some nice bash magic that makes them whitespace-safe. |
50 |
|
51 |
>+ local f="${!i}" |
52 |
>+ if [ "${f#-${findflag#-}}" != "${f}" ] ; then |
53 |
|
54 |
I know the original code sucked as well but could you replace this with more readable [[ ${f} == -${findflag#-}* ]] or alike (note: not tested). |
55 |
|
56 |
>+ printf "%s\n" "${f#-${findflag}=}" |
57 |
|
58 |
It may be a good idea to add a short explanation why you can't use echo here, as a comment. |
59 |
|
60 |
> return 0 |
61 |
> fi |
62 |
>+ ((i--)) |
63 |
> done |
64 |
> done |
65 |
> return 1 |
66 |
|
67 |
|
68 |
-- |
69 |
Best regards, |
70 |
Michał Górny (by phone) |