labath added a comment.
In D81001#2162743 <https://reviews.llvm.org/D81001#2162743>, @gedatsu217 wrote:
> In addition to it, I tried the below code, but it did not go well. ("\x1b[nD"
> moves the cursor n steps to the left.)
>
> self.child.send("hel")
> self.child.expect_exact(faint_color + "p frame" + reset + "\x1b[" +
> str(len("p frame")) + "D")
>
>
> In the first place, the below code does not go well.
>
> self.child.send("help frame")
> self.child.expect_exact("help frame" + "\x1b[0D")
>
>
> "\x1b[0D" means that the cursor does not move. So, I suspect "\x1b[nD" does
> not function in pexpect. What do you think?
It definitely "functions", but it's possible that lldb/editline output a
slightly different sequence with the same effect. For example it might output
"\x1b[mG", which would move the cursor to the absolute column `m`. Other ways
of achieving the same thing are possible, but they are more convoluted so I
don't expect we are using them. Bottom line: You'll need to check what's the
sequence that actually gets output.
================
Comment at: lldb/source/Core/IOHandler.cpp:204
+ .GetAutoSuggestionForCommand(line))
+ result = res.getValue();
+}
----------------
gedatsu217 wrote:
> labath wrote:
> > gedatsu217 wrote:
> > > labath wrote:
> > > > Why does this switch Optional<string> result to a by-ref string
> > > > argument. We have both code which uses an empty string to signify
> > > > failure, and code which does that with `None`. Both have their
> > > > advantages and disadvantages, and I don't have any strong objections to
> > > > either one, but I certainly don't see an advantage in using both in the
> > > > same patch.
> > > Sorry, I do not what you mean. Would you explain it more in detail?
> > >
> > > (If None is returned, it is not assigned to "result". So, only is string
> > > assigned to "result". Does this answer your question?)
> > Ok, let me try to phrase that differently:
> > - if `GetAutoSuggestionForCommand` fails, it returns `None`
> > - if `IOHandlerDelegate::IOHandlerSuggestion` fails, it sets a by-ref
> > string argument to `""` (or rather, leaves it as empty).
> >
> > The two behaviors are different, and it's not clear to me what justifies
> > that difference. It would be much cleaner and clearer if both were using
> > the same convention. Changing conventions part way like this is confusing
> > and forces one to add conversion code. If `IOHandlerSuggestion` was using
> > the same convention, then it's implementation would just be `return
> > io_handler.GetDebugger().GetCommandInterpreter().GetAutoSuggestionForCommand(line))`.
> I understood it.
> Even if I change return of IOHandlerSuggestion from void to
> llvm::Optional<std::string>, I think I have to convert None to empty string
> in another function eventually, because the final perpose of these function
> is to make string for autosuggestion.
>
> Therefore, if I change return of
> CommandInterperter::GetAutoSuggestionForCommand from
> llvm::Optional<std::string> to just std::string, I think above problem will
> be solved. What do you think?
Are you sure about that? If I follow the code correctly this eventually gets
called in `Editline::ApplyAutosuggestCommand`
```
std::string to_add = "";
m_suggestion_callback(line, to_add, m_suggestion_callback_baton);
if (to_add.empty())
return CC_REFRESH;
el_insertstr(m_editline, to_add.c_str());
return CC_REDISPLAY;
```
That could be written as:
```
if (Optional<string> to_add = m_suggestion_callback(line, to_add,
m_suggestion_callback_baton)) {
el_insertstr(m_editline, to_add->c_str());
return CC_REDISPLAY;
}
return CC_REFRESH;
```
which is actually shorter and makes the failure case more explicit.
The call in `Editline::TypedCharacter` could be modified similarly...
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
