Add tab completion for file paths in save/open prompts #323
Conversation
babi/prompt.py
Outdated
| self, | ||
| screen: Screen, | ||
| prompt: str, lst: list[str], | ||
| file_glob: bool = False, |
There was a problem hiding this comment.
this is a boolean trap -- it should be named-only if anything
There was a problem hiding this comment.
Sorry, not sure I understand what exactly you'd like me to do here.
Something like lst: list[str], *, file_glob: bool[ = False]?
Do you want me to drop the default value as well?
[Also realised I forgot a line break between prompt and lst]
There was a problem hiding this comment.
hi, I hope my comment doesn't come out of place -- anthony made a video on boolean traps
babi/prompt.py
Outdated
| self._dispatch = Prompt.DISPATCH.copy() | ||
| if file_glob: | ||
| self._dispatch[b'^I'] = Prompt._file_complete |
There was a problem hiding this comment.
unknown keys are already ignored in prompts -- the completion callback should just check if it's enabled or not rather than having to copy this dict every time
There was a problem hiding this comment.
I don't think I understand this point - a key seems to be considered "known" as soon as it's in the dispatch dict, so I can't just add it globally.
If I didn't copy the dict, I'd be modifying the global DISPATCH dict, AFAIUI?
Also, for the callback to check on whether globbing is enabled, i'd have to make file_glob an instance variable, correct?
There was a problem hiding this comment.
I'm saying you should handle the option inside the callback
| @@ -95,6 +105,17 @@ def _delete(self) -> None: | |||
| def _cut_to_end(self) -> None: | |||
| self._s = self._s[:self._x] | |||
|
|
|||
| def _file_complete(self) -> None: | |||
| partial = self._s[:self._x] | |||
There was a problem hiding this comment.
this doesn't seem quite right -- it's going to take imagine foo.txt is on disk and you've written fo][od and you tab you're going to get foo.txtod which is nonsensical. I think it should really only listen to tab if you're at the end of the string
There was a problem hiding this comment.
Sometimes, this is what I want, and I use this with tools that allow it.
Example: so][/foo.txt -> someLongFolder/foo.txt
This is especially useful to me to create a new file in an existing folder.
I understand this is a question of preference, I'd personally prefer this behaviour to stay as is.
[I usually add a space to the right of the cursor before hitting tab for visual separation and then delete it after completion has done its job, but that uses the same mechanic. I doubt we'd want to check for whitespace after the cursor and make it conditional]
There was a problem hiding this comment.
in that case it could listen to tab if it is at the end of the string or before a path separator I guess?
| filename = self.prompt( | ||
| 'enter filename', default=self.file.filename, file_glob=True, | ||
| ) |
There was a problem hiding this comment.
this changes the behaviour. these shouldn't have a default
InsanePrawn
force-pushed
the
prawn/fileglob
branch
from
3d43be3
to
0541559
Compare
| screen: Screen, | ||
| prompt: str, lst: list[str], | ||
| *, | ||
| file_glob: bool, |
There was a problem hiding this comment.
this is maybe not a good name -- it's really a flag for whether tab completion of files occurs -- and can probably default to False
| @@ -165,6 +192,7 @@ def _submit(self) -> str: | |||
| b'KEY_DC': _delete, | |||
| b'^K': _cut_to_end, | |||
| # misc | |||
| b'^I': _tab, | |||
| base = 'globtest' | ||
| prefix = base + 'ffff.txt' |
There was a problem hiding this comment.
rather than variables I'd rather see the actual strings in the assertions -- rather than having to hunt around for what is actually being tested the expressions should be clear in tests. https://testing.googleblog.com/2014/07/testing-on-toilet-dont-put-logic-in.html
No description provided.