ruiu added a comment.
As to the order of Command variable contents, I think I'm not convinced. You
can access the last element as `Command[Command.size() - 1]` (or maybe
`Command.back()`), no? It is slightly awkward than `Command[0]`, but that's not
horribly hard to handle, and passing arguments in the reverse order seems more
counter-intuitive to me.
================
Comment at: clang/include/clang/Driver/Options.td:2198
def stdlib_EQ : Joined<["-", "--"], "stdlib=">, Flags<[CC1Option]>,
- HelpText<"C++ standard library to use">;
+ HelpText<"C++ standard library to use">, Values<"libc++,libstdc++,platform">;
def sub__library : JoinedOrSeparate<["-"], "sub_library">;
----------------
v.g.vassilev wrote:
> ruiu wrote:
> > v.g.vassilev wrote:
> > > `Values` seems too generic, can we use `ArgValues` instead?
> > I'd keep it as `Values`, as everything is essentially related to command
> > line arguments, and `Args` seems redundant. For example, we didn't name
> > `ArgFlags` but just `Flags`, and `HelpText` instead of `ArgHelpText`, etc.
> My reasoning for asking this is that I wanted to hint about the relationship
> between the values (as this is a very broad term) and arguments. I'd read
> `ArgValues` as possible values for an argument without having to dig into
> context.
>
> That said, I don't think switching back to `Values` is a significant issue,
> if @ruiu feels strongly about it , please follow his suggestion and land the
> patch.
I think I feel fairly strongly prefer "Value" over "ArgValue". If this is a
one-time name, I agree that we should use "ArgValue", but this is not the case.
We are going to add bunch of "Values" to options that take arguments, and I
expect that the verbose name will start feeling too verbose. So please switch
back to "Values".
================
Comment at: clang/utils/bash-autocomplete.sh:7
- flags=$( clang --autocomplete="$cur" )
- if [[ "$flags" == "" || "$cur" == "" ]]; then
+ arg=""
+ for (( i = $cword; i >= 1; i-- )) ;
----------------
Don't you want to make it `local`?
================
Comment at: clang/utils/bash-autocomplete.sh:8
+ arg=""
+ for (( i = $cword; i >= 1; i-- )) ;
+ do
----------------
If you don't write `do` on the same line as `for`, you don't need to write `;`
at end of a line. I.e. write it in either
for ...
do
command...
done
or
for ...; do
command...
done
This is because you need a newline between `for` and `do`, and `;` works as a
newline.
================
Comment at: clang/utils/bash-autocomplete.sh:10
+ do
+ arg="$arg""${COMP_WORDS[$i]},"
+ done
----------------
nit: you don't need double double-quotes. Instead, `"$arg${COMP_WORDS[$i]},"
should just work.
================
Comment at: clang/utils/bash-autocomplete.sh:15-16
+ if [[ "$cur" == "=" ]]; then
+ cur=""
+ COMPREPLY=( $( compgen -W "$flags" -- "$cur") )
+ else if [[ "$flags" == "" || "$cur" == "" ]]; then
----------------
Alternatively, `COMPRELY=( $( compgen -W "$flags" -- "") )` should work.
================
Comment at: clang/utils/bash-autocomplete.sh:17
+ COMPREPLY=( $( compgen -W "$flags" -- "$cur") )
+ else if [[ "$flags" == "" || "$cur" == "" ]]; then
_filedir
----------------
Use `elif` so that you don't have to write two `fi`s at end. In a bash script,
if ... else if is usually written as
if ...; then
elif ...; then
elif ...; then
fi
If you use `else if` instead of `elif`, and if you indent the code properly,
it'll look like
if ...; then
else
if ...; then
fi
fi
This is why you needed two `fi`s.
https://reviews.llvm.org/D33383
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
