[Lldb-commits] [PATCH] D114448: [Bug 49018][lldb] Fix incorrect help text for 'memory write' command

[Venkata Ramanaiah Nalamothu via Phabricator via lldb-commits]

RamNalamothu added a comment.

In D114448#3150900 <https://reviews.llvm.org/D114448#3150900>, @DavidSpickett wrote:

> Not sure if you're a regular contributor and already know this but in case not, I tend to use https://clang.llvm.org/docs/ClangFormat.html#script-for-patch-reformatting for doing clang-format stuff. It'll format only the lines you've changed.

Thanks for sharing that. I normally use `git-clang-format` (i.e. clang-format extension for git) which does the same job.



================
Comment at: lldb/source/Interpreter/OptionGroupFormat.cpp:66
+           0,
+           eArgTypeCount,
+           "The number of total items to display."},
----------------
DavidSpickett wrote:
> Not sure if clang-format has done this, or just your preferred style. Nothing against either but it makes it difficult to tell if anything has changed in this particular part of the diff.
> 
> In general we want clang-formatted code but if it makes the diff tricky to read it's best done in a follow up change that just does formatting.
The clang-format did it. Will mark this with `clang-format off/on`.


================
Comment at: lldb/source/Interpreter/OptionGroupFormat.cpp:73
+      case eArgTypeFormat:
+        m_format_usage_text.append(std::get<1>(usage_text_tuple));
+        m_option_definition[0].usage_text = m_format_usage_text.data();
----------------
DavidSpickett wrote:
> As I said above, if this is about commands being able to *replace* the help text I don't think append is needed here.
> 
> Unless std::string is helping you check whether it's been set or not, which I think you could do with nullptr just as well but please correct me if I'm missing some context.
I kind of took the std::string approach from `OptionGroupPythonClassWithDict` but as you mentioned it is not needed and will fix that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114448