[Lldb-commits] [PATCH] D77444: [commands] Support autorepeat in SBCommands

2020-04-09 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments. Comment at: lldb/unittests/API/CMakeLists.txt:2 +add_lldb_unittest(APITests + TestSBCommandInterpreterTest.cpp + Well.. this is now definitely going to get tested well. :P While we do have some inconsistency where some test files

[Lldb-commits] [PATCH] D77444: [commands] Support autorepeat in SBCommands

2020-04-08 Thread Phabricator via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGbe3f8a8e1b95: [commands] Support autorepeat in SBCommands (authored by Walter Erquinigo wall...@fb.com). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D77444: [commands] Support autorepeat in SBCommands

2020-04-08 Thread Jim Ingham via Phabricator via lldb-commits
jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77444/new/ https://reviews.llvm.org/D77444

[Lldb-commits] [PATCH] D77444: [commands] Support autorepeat in SBCommands

2020-04-08 Thread walter erquinigo via Phabricator via lldb-commits
wallace updated this revision to Diff 256027. wallace added a comment. address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77444/new/ https://reviews.llvm.org/D77444 Files: lldb/include/lldb/API/SBCommandInterpreter.h

[Lldb-commits] [PATCH] D77444: [commands] Support autorepeat in SBCommands

2020-04-07 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. This looks good to me, except that instead of leaving all the other variants of AddCommand, they should funnel through the one that takes the most arguments. It was poor form to leave two around and more so now that there's three. I'm beginning to think we need an

[Lldb-commits] [PATCH] D77444: [commands] Support autorepeat in SBCommands

2020-04-07 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. BTW, in gtest tests, the prevailing convention is to name test .cpp files according to the name of the .cpp file under test. So, in this case, that would be something like `unittests/API/SBCommandInterpreterTest.cpp`. I wouldn't want to take that convention to the

[Lldb-commits] [PATCH] D77444: [commands] Support autorepeat in SBCommands

2020-04-07 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. Thank you for trying out gtest. I'm very happy that this is working out for you. The test looks fine -- just please move it to `unittests/API` (new folder) -- that's where it logically belongs as it's testing the API code (and it also avoids dangerous ODR violations

Re: [Lldb-commits] [PATCH] D77444: [commands] Support autorepeat in SBCommands

2020-04-06 Thread Walter via lldb-commits
> BTW, I thought when a command returned the command string for the next command, the command interpreter prepended it with the chain of parent commands containing the command that was presenting the “next command string”. That is correct. The GetRepeatCommand function gets the full command with

Re: [Lldb-commits] [PATCH] D77444: [commands] Support autorepeat in SBCommands

2020-04-06 Thread Jim Ingham via lldb-commits
Then only time that I can find where I customized the string based on the incoming full command was for “source list” because I needed to know whether the listing was going forward or backward, so I needed to write that into the “keep doing what you were doing” command. That could have also

[Lldb-commits] [PATCH] D77444: [commands] Support autorepeat in SBCommands

2020-04-06 Thread walter erquinigo via Phabricator via lldb-commits
wallace updated this revision to Diff 255429. wallace added a comment. nits Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77444/new/ https://reviews.llvm.org/D77444 Files: lldb/include/lldb/API/SBCommandInterpreter.h

[Lldb-commits] [PATCH] D77444: [commands] Support autorepeat in SBCommands

2020-04-06 Thread walter erquinigo via Phabricator via lldb-commits
wallace updated this revision to Diff 255426. wallace added a comment. Herald added a subscriber: mgorny. - Moved the test to gtest. It's much better this way and I learned gtest - Changed the API. Some notes: Within the scope of a subcommand, the subcommand doesn't know the parent's command

[Lldb-commits] [PATCH] D77444: [commands] Support autorepeat in SBCommands

2020-04-06 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment. In D77444#1963354 , @labath wrote: > In D77444#1962952 , @clayborg wrote: > > > The biggest issue is maintaining the API. We can't add anything to > > SBCommandPluginInterface > > > If

[Lldb-commits] [PATCH] D77444: [commands] Support autorepeat in SBCommands

2020-04-06 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In D77444#1962952 , @clayborg wrote: > The biggest issue is maintaining the API. We can't add anything to > SBCommandPluginInterface If we're going to be adding a new inheritable class for this feature, I'd recommend adding

[Lldb-commits] [PATCH] D77444: [commands] Support autorepeat in SBCommands

2020-04-05 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment. I was thinking if auto repeat was enabled that the DoExecute would just get called again with nothing and then the plug-in could maintain any repeat commands without needing to add anything... Glad you chimed in! Repository: rG LLVM Github Monorepo CHANGES SINCE

[Lldb-commits] [PATCH] D77444: [commands] Support autorepeat in SBCommands

2020-04-05 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment. The biggest issue is maintaining the API. We can't add anything to SBCommandPluginInterface Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77444/new/ https://reviews.llvm.org/D77444

[Lldb-commits] [PATCH] D77444: [commands] Support autorepeat in SBCommands

2020-04-03 Thread walter erquinigo via Phabricator via lldb-commits
wallace planned changes to this revision. wallace added a comment. Very good idea! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77444/new/ https://reviews.llvm.org/D77444 ___ lldb-commits mailing

[Lldb-commits] [PATCH] D77444: [commands] Support autorepeat in SBCommands

2020-04-03 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. For instance, maybe instead of passing in a bool, you could pass in the repeat command? So when you create the command, if you pass in None, you get no repeat, if you pass in "" you get the exact command line repeated, and if you pass in anything else, that will be

[Lldb-commits] [PATCH] D77444: [commands] Support autorepeat in SBCommands

2020-04-03 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. It's great to add this capability. But if you pass a nullptr, then the exact previous command will be repeated. That's sometimes what you want, but often not. For instance: (lldb) source list -f foo -l 10 ... (lldb) You DON'T want those same source lines to be

[Lldb-commits] [PATCH] D77444: [commands] Support autorepeat in SBCommands

2020-04-03 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment. LGTM. Pavel? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77444/new/ https://reviews.llvm.org/D77444 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D77444: [commands] Support autorepeat in SBCommands

2020-04-03 Thread walter erquinigo via Phabricator via lldb-commits
wallace created this revision. wallace added reviewers: labath, clayborg. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. This adds support for commands created through the API to support autorepeat. This covers the case of single word and multiword commands. Comprehensive