grimar added a comment. In D56014#1339419 <https://reviews.llvm.org/D56014#1339419>, @davide wrote:
> testcase? I investigated how to add it and I do not see a good way write a test :( Seems the proper place to test it would be `unittest/Editline`: https://github.com/llvm-mirror/lldb/blob/master/unittests/Editline/EditlineTest.cpp It allows to send the text using the virtual terminal: https://github.com/llvm-mirror/lldb/blob/master/unittests/Editline/EditlineTest.cpp#L294 And I was able to simulate the key up press ("lldb-previous-line" command) sending the "\x1b[A" line and to trigger the call of the private `RecallHistory` method of `Editline`, where this patch did the fix. (Since we do not want to make methods public just for the test I believe) But this is not enough. Case fixed by this patch assumes that some expression history exists. Class responsible for work with the history is `EditlineHistory`: https://github.com/llvm-mirror/lldb/blob/master/source/Host/common/Editline.cpp#L162 It contains logic to find the path for history file and logic to load/save it with the editline library API. This class is a privately implemented in Editline.cpp and used internally by `Editline`. I am not sure it is a good idea to export it for the unit test. I do not think it is correct to duplicate its logic either or to create a history file manually, given that it seems to contain a mini header, probably specific to the editline library version. And also (perhaps that would not be needed, but FTR) it does not seem to be a good idea to use the editline API in EditlineTest.cpp, I think it should remain encapsulated in Editline.cpp, like now. Given all above it seems it is really problematic to write a test for this now. My suggestion probably is to go without it (given that patch fixes an obvious mistype). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56014/new/ https://reviews.llvm.org/D56014 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
