This is an archive of the discontinued LLVM Phabricator instance.

[lldb/Commands] Fix disk completion from root directory
ClosedPublic

Authored by mib on Jun 2 2023, 10:41 AM.

Details

Summary

This patch should fix path completion starting from the root directory.

To do so, this patch adds a special case when setting the search
directory when the completion buffer points to the root directory.

Signed-off-by: Med Ismail Bennani <ismail@bennani.ma>

Diff Detail

Event Timeline

mib created this revision.Jun 2 2023, 10:41 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 2 2023, 10:41 AM
mib requested review of this revision.Jun 2 2023, 10:41 AM

The change itself looks ok to me. Could you add a test for this? We do have some tests to test for completion so the machinery is there, but it's mostly for the expression evaluator right now.

mib updated this revision to Diff 528058.Jun 2 2023, 6:13 PM

Add test

bulbazord added inline comments.Jun 2 2023, 11:23 PM
lldb/test/API/functionalities/completion/TestCompletion.py
439–450

I'm curious to know if this will work for windows? I don't know how lldb treats / as a path when on a windows host.

root_dir should be os.path.abspath('\\') on windows, which might give you C:\ or whatever the current drive is, so that path might possibly give you something? Idk, you might have to fiddle with the test to get it to work correctly or disable it.

mib marked an inline comment as done.Jun 3 2023, 12:19 AM
mib added inline comments.
lldb/test/API/functionalities/completion/TestCompletion.py
439–450

I don't have a windows machine to try this on (some help would be appreciated here :p) but os.path.abspath(os.sep) should cover that part.

mib updated this revision to Diff 528560.Jun 5 2023, 1:20 PM
mib marked an inline comment as done.

Address @bulbazord comments!

bulbazord accepted this revision.Jun 5 2023, 1:21 PM

Ok, this looks fine to me. I'm not 100% sure this will work on Windows, but we neither Ismail nor I have windows machines to test this on. Watch the windows bots please.

This revision is now accepted and ready to land.Jun 5 2023, 1:21 PM
JDevlieghere accepted this revision.Jun 5 2023, 1:28 PM
This revision was landed with ongoing or failed builds.Jun 6 2023, 10:59 AM
This revision was automatically updated to reflect the committed changes.