Re: [Lldb-commits] [PATCH] D17724: Replace getopt with llvm::cl in lldb driver

2016-03-03 Thread Greg Clayton via lldb-commits
> On Mar 3, 2016, at 12:33 PM, Jim Ingham wrote: > > >> On Mar 2, 2016, at 10:18 AM, Greg Clayton wrote: >> >> So this is nice that you cleaned up this one place where we can use LLVM's >> option parsing stuff, but it now makes the driver inconsistent

Re: [Lldb-commits] [PATCH] D17724: Replace getopt with llvm::cl in lldb driver

2016-03-03 Thread Jim Ingham via lldb-commits
> On Mar 2, 2016, at 10:18 AM, Greg Clayton wrote: > > So this is nice that you cleaned up this one place where we can use LLVM's > option parsing stuff, but it now makes the driver inconsistent with the rest > of LLDB. It also makes it so you have to link some LLVM .a

Re: [Lldb-commits] [PATCH] D17724: Replace getopt with llvm::cl in lldb driver

2016-03-03 Thread Greg Clayton via lldb-commits
clayborg added a comment. Feel free to check this in a branch if you want to keep it around for when you later work on this so you don't lose the changes? http://reviews.llvm.org/D17724 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

Re: [Lldb-commits] [PATCH] D17724: Replace getopt with llvm::cl in lldb driver

2016-03-03 Thread Pavel Labath via lldb-commits
labath abandoned this revision. labath added a comment. Ok, fair enough. I'll see if I can do something like that in the future, but I'm going to abandon this for the moment. http://reviews.llvm.org/D17724 ___ lldb-commits mailing list

Re: [Lldb-commits] [PATCH] D17724: Replace getopt with llvm::cl in lldb driver

2016-03-02 Thread Kamil Rytarowski via lldb-commits
krytarowski added a comment. In http://reviews.llvm.org/D17724#366479, @clayborg wrote: > Our current consistent way of parsing options is to use getopt_long() > everywhere. Actually with usage of `getopt_long_only()` which is unportable. http://reviews.llvm.org/D17724

Re: [Lldb-commits] [PATCH] D17724: Replace getopt with llvm::cl in lldb driver

2016-03-02 Thread Greg Clayton via lldb-commits
clayborg added a comment. Our current consistent way of parsing options is to use getopt_long() everywhere. http://reviews.llvm.org/D17724 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

Re: [Lldb-commits] [PATCH] D17724: Replace getopt with llvm::cl in lldb driver

2016-03-02 Thread Greg Clayton via lldb-commits
clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. So this is nice that you cleaned up this one place where we can use LLVM's option parsing stuff, but it now makes the driver inconsistent with the rest of LLDB. It also makes it

Re: [Lldb-commits] [PATCH] D17724: Replace getopt with llvm::cl in lldb driver

2016-03-02 Thread Pavel Labath via lldb-commits
labath added a subscriber: jingham. labath added a comment. Greg, Jim, any objections? http://reviews.llvm.org/D17724 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D17724: Replace getopt with llvm::cl in lldb driver

2016-03-01 Thread Zachary Turner via lldb-commits
I think there's a lot of value in having lldb's help output and cl syntax match all of the other llvm tools that people are already familiar with. Simplicity through consistency. So lgtm On Tue, Mar 1, 2016 at 4:22 AM Pavel Labath wrote: > labath added a comment. > > In

Re: [Lldb-commits] [PATCH] D17724: Replace getopt with llvm::cl in lldb driver

2016-03-01 Thread Zachary Turner via lldb-commits
zturner added a comment. I think there's a lot of value in having lldb's help output and cl syntax match all of the other llvm tools that people are already familiar with. Simplicity through consistency. So lgtm http://reviews.llvm.org/D17724 ___

Re: [Lldb-commits] [PATCH] D17724: Replace getopt with llvm::cl in lldb driver

2016-03-01 Thread Pavel Labath via lldb-commits
labath added a comment. In http://reviews.llvm.org/D17724#364450, @clayborg wrote: > As long as both long and short options are still supported? Can you still > type any of: > > - --arch=x86_64 > - --arch x86_64 > - -arch=x86_64 > - -arch x86_64 > - -ax86_64 > - -a x86_64 All the variants

Re: [Lldb-commits] [PATCH] D17724: Replace getopt with llvm::cl in lldb driver

2016-02-29 Thread Jim Ingham via lldb-commits
The straightforward way to do specify a complex command line for debugging is: lldb —arch=x86_64 — /bin/ls -IAF everything after the — are arguments to the program. This also allows you to cut & paste whole command lines. It works by the trick that if you haven’t specified a file before the

Re: [Lldb-commits] [PATCH] D17724: Replace getopt with llvm::cl in lldb driver

2016-02-29 Thread Greg Clayton via lldb-commits
clayborg added a comment. In http://reviews.llvm.org/D17724#364454, @zturner wrote: > long and short options are supported, but the one I'm not sure about is the > case where you use a short option with no space. Your example "-ax86_64" > might not work. It might, just that it should be

Re: [Lldb-commits] [PATCH] D17724: Replace getopt with llvm::cl in lldb driver

2016-02-29 Thread Zachary Turner via lldb-commits
long and short options are supported, but the one I'm not sure about is the case where you use a short option with no space. Your example "-ax86_64" might not work. It might, just that it should be tested. I'm 99% confident the rest of them all work. Also not sure about this example: "% lldb

Re: [Lldb-commits] [PATCH] D17724: Replace getopt with llvm::cl in lldb driver

2016-02-29 Thread Greg Clayton via lldb-commits
clayborg added a comment. As long as both long and short options are still supported? Can you still type any of: - --arch=x86_64 - --arch x86_64 - -arch=x86_64 - -arch x86_64 - -ax86_64 - -a x86_64 Are we able to track which options can be used with other options with the llvm solution? I

Re: [Lldb-commits] [PATCH] D17724: Replace getopt with llvm::cl in lldb driver

2016-02-29 Thread Pavel Labath via lldb-commits
labath added a comment. Yes, we are, and unfortunately we can't get rid of it that easily there, as llvm:cl does not really have interactive uses in mind (global variables everywhere, --help kills your program, etc.). It probably wouldn't be **too** hard to add a version which can do that, as

Re: [Lldb-commits] [PATCH] D17724: Replace getopt with llvm::cl in lldb driver

2016-02-29 Thread Zachary Turner via lldb-commits
Love this change in principle. I will look over it on the windows side to make sure nothing breaks. One question: we are using getopt still for parsing interactive command lines right? On Mon, Feb 29, 2016 at 9:26 AM Pavel Labath wrote: > labath created this revision. >

[Lldb-commits] [PATCH] D17724: Replace getopt with llvm::cl in lldb driver

2016-02-29 Thread Pavel Labath via lldb-commits
labath created this revision. labath added reviewers: clayborg, zturner. labath added a subscriber: lldb-commits. This replaces the hand-rolled getopt option parser in lldb driver with the one in llvm. This results in a lot less code, as the llvm's parser does much of the work (e.g., formatting