[Lldb-commits] [PATCH] D88129: Add "break delete --disabled" to delete all currently disabled breakpoints

[Jim Ingham via Phabricator via lldb-commits]

jingham added inline comments.


================
Comment at: lldb/source/Commands/Options.td:232
+    Desc<"Delete all breakpoints which are currently disabled.  When using the disabled option "
+    "any breakpoints listed on the command line are EXCLUDED from deletion.">;
 }
----------------
kastiglione wrote:
> jingham wrote:
> > kastiglione wrote:
> > > jingham wrote:
> > > > kastiglione wrote:
> > > > > To me, it's counter intuitive that `break delete --disabled 1` will not delete bp 1.
> > > > The combination:
> > > > 
> > > > (lldb) break delete --disabled 1
> > > > 
> > > > could either mean 
> > > > 
> > > > 1) delete all breakpoints that are disabled AND breakpoint 1
> > > > 2) delete all breakpoints that are disabled EXCEPT breakpoint 1
> > > > 3) an error
> > > > 
> > > > Of those interpretations, 1 and 3 don't seem very useful, but 2 does.  
> > > > 
> > > > This is particularly handy when you specify a breakpoint name, not a breakpoint.  Just make breakpoints you don't want deleted DoNotDelete, then you can easily protect all those breakpoints.  
> > > > 
> > > > Note, your workaround would only be useful in this case if all the breakpoints named DoNotDelete are currently disabled.  Otherwise you would have to remember which of the DoNotDelete breakpoints were disabled, enable them all, do the `delete --disabled` then  only re-disable those that were originally disabled.  Whereas if you can pass an exclude list you can just protect those breakpoints unconditionally regardless of their state.
> > > > 
> > > > So while I agree this is a little odd, it's actually the only option that really makes sense, it's easy to document, and I don't think it's likely to cause mistakes. 
> > > why does the first interpretation not seem useful? If I'm deleting breakpoints, I might want to delete both disabled breakpoints and one or more specific breakpoints. To do that I would probably intuitively write `break delete --disabled OthersToDelete`.
> > > 
> > > Could the ambiguity be removed by adding another flag? `break delete --disabled --except DoNotDelete`?
> > To me "delete --disabled" is a bulk operation acting on a class of breakpoints.  "This class plus one random other one" seems odd to me.  
> > 
> > A bulk operation with exclusions makes much more sense to me.  
> > 
> > Adding another option complicates things without adding much value, and becomes annoying if you want to specify more than one excluded thing.  It would be easy to make the mistake:
> > 
> > (lldb) break disable --disabled --except 1 2
> > 
> > intending to preserve 2 but in fact deleting it.
> I get that exclusions are useful, my concern is that the command "breakpoint delete" doesn't delete what you give it. If `break delete foo` deletes foo, then on the surface `break delete --disabled foo` should also delete foo. The flag does what it says, but also silently inverts the meaning of the positional args.
The help for the option explicitly says that it inverts the meaning of the positional args, there's nothing silent about it.  You wouldn't accidentally say `break delete --disabled`, so presumably you would have to have read the help for the option, which I don't think is susceptible to misconstruction.

Because of that, I'm not too bothered that `break delete --disabled Foo` behaves differently from `break delete Foo`.  And it seems the simplest way to express the most useful thing you would want to add to just`break delete --disable`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88129/new/

https://reviews.llvm.org/D88129