teemperor added a comment. In D81001#2107102 <https://reviews.llvm.org/D81001#2107102>, @gedatsu217 wrote:
> Change the name and return of the function( bool UseAutosuggestion() -> void > SetShowAutosuggestion (bool) ) (ll. 1447 in Editline.cpp and ll.194 in > Editline.h). > int -> size_t (ll. 1009 in Editline.cpp). > Fix for normal setter (ll. 269 in IOHandler.cpp). > > > The Editline instance isn't recreated when you change the settings, so you > > need to restart LLDB for this to work. However, settings aren't > > automatically saved, so I think the right way to do this by putting the > > settings set show-autosuggestion true into the ~/.lldbinit file (which will > > be automatically executed before LLDB's command line frontend starts). It > > probably requires a bunch of work to get that working without a restart, so > > I think this can be fixed as a follow-up. > > There is no ~/.lldbinit in my environment (I do not why). Instead, there is > ~/.lldb directory (but there are just command history file there.). You need to create that file :) It's just a text file with LLDB commands in each line (and you just put the settings command in there to change the setting before LLDB starts). Doing `lldb -o "settings set show-autosuggestion true"` should also work. >> On a more general note: I'm not sure why we need m_current_autosuggestion. >> There is a bunch of code that tries to keep that variable up-to-date with >> what is typed, but unless I'm missing something this is just the text the >> user has entered so far? If yes, then you can also just get the current user >> input from Editline (see the first few lines of the Editline::TabCommand >> function for how to do that). > > I think "m_current_autosuggestion" is needed. That is because it keeps the > characters for the suggestion, not just user input. For example, when "b" is > typed, "reakpoint" is saved in m_current_autosuggestion (when "breakpoint" is > typed before). > > When a character is typed, Editline::TypedCharacter is called and > m_current_autosuggestion is renewed every time. On the other hand, > Editline::ApplyCompleteCommand, which execute suggestion actually, is not > called unless C^f is typed. Therefore I think the suggestion parts should be > saved until it is called. Indeed, I can get current user input by the first > few lines of the Editline::TabCommand function, but it cannot save the > suggestion parts probably. > > However, I noticed that "to_add" in Editline::TypedCharacter is unnecessary, > so removeed it. My worry is that if we rely on always keeping this variable up-to-date during the life-time of Editline that this might lead to weird bugs due to the variable getting out-of-sync (e.g., someone changes some input logic but forgets to update the autosuggestion code). I tried moving some parts in this patch around in this commit <https://github.com/Teemperor/llvm-project/compare/arcpatch-D81001...Teemperor:AutosuggestionNoMemberVar> and it does seem to work in the same way as this patch, but without the need to keep m_current_autosuggestion up-to-date. What do you think? ================ Comment at: lldb/source/Host/common/Editline.cpp:1451 + else + m_use_autosuggestion = false; +} ---------------- This can all be `m_use_autosuggestion = autosuggestion;`. ================ Comment at: lldb/source/Interpreter/CommandInterpreter.cpp:1875-1878 + if (entry.startswith(line)) { + llvm::StringRef res = entry.substr(line.size()); + result = res.str(); + return result; ---------------- gedatsu217 wrote: > labath wrote: > > ``` > > if (entry.consume_front(line)) { > > result = entry.str(); > > return result; // Why is this returning the result both as a proper > > return value and a by-ref argument? > > } > > ``` > That is because using the llvm::Optional as a return value instead of void > would make it more obvious what the function returns in case there is no > suggestion. > > We've discussed this issue before, so please see the comments above. Pavel's point is that the function is returning >both<. You can just remove the `result` parameter and do `result = ... GetAutoSuggestionForCommand(line);` when you call it is his point. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81001/new/ https://reviews.llvm.org/D81001 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
