Provide libedit a special function making it always add a space after
the autocompleted command. The default one adds a slash if the word is
also a name of a directory in the current working directory, but this is
wrong for commands.
Details
Details
Diff Detail
Diff Detail
- Lint
Lint Skipped - Unit
Tests Skipped
Event Timeline
| bin/sh/expand.c | ||
|---|---|---|
| 365 | Passing EXP_SPLIT, EXP_GLOB or EXP_CASE will generate internal representation that the caller will probably not be able to use. Since the (only) caller does not need any flags, I would prefer not having a flags parameter in tilde_expand() at all. | |
| bin/sh/histedit.c | ||
| 721 | Since both tilde_expand and the parser use STARTSTACKSTR and the like to build strings, this needs some extra code to prevent them from interfering. Otherwise, things could go wrong if completion is done while reading a multi-line string or a here-document. The grabstackstr()/ungrabstackstr() pair suggested by comments in memalloc.c is not attractive here, since it would slow down all parsing (including of scripts) for an interactive feature. Perhaps it is better to make it such that the code called here will call malloc() for a new stack block. | |
| bin/sh/expand.c | ||
|---|---|---|
| 365 | I knew the only caller didn't make any use of the flags argument, but I thought it might come in handy in the future and it didn't look like much work to make a caller be able to interpret the internal representation. But I don't have strong feelings about it, it can be revisited if ever needed. | |
| bin/sh/histedit.c | ||
| 721 | Can you help me understand under what circumstances this interactive-mode-only code could interfere with the parser performance (if we added grabstackstr()) or behavior (STARTSTACKSTR use)? Do you mean those could happen during script execution? | |
| bin/sh/histedit.c | ||
|---|---|---|
| 721 | the ,0 is a left over? | |
| bin/sh/histedit.c | ||
|---|---|---|
| 721 | Scenario: start sh, type <<X some text ~/ then press Tab. As it is now, readtoken1() in parser.c builds up the text of the here-document in the stack string, using a local variable (out) to point to the location for the next byte. If the line editing code (called from there via pgetc() and variants) also uses the stack string, it will overwrite this data with the tilde-expanded completion word. Some other code in this situation uses grabstackstr() and ungrabstackstr() to "allocate" the space, so the tilde expansion will not overwrite it. However, doing that here in the straightforward way would involve a grabstackstr()/ungrabstackstr() pair for most bytes parsed by the shell, greatly slowing down the parser (I have not benchmarked this). Making out a global variable is also likely to lead to a measurable slowdown. An approach that may work is to pierce the abstraction and only grabstackstr()/ungrabstackstr() when pgetc_macro() cannot obtain a byte from the buffer. Another approach may be to move the problem entirely to the completion code, and have that save and restore the state of the stack string so it does not interfere with the parser. Since it is not known how much space readtoken1() is using in the current block (tracked in the local variable out), the completion code cannot use the block at all. I have not thought out the details of this, but it seems most promising. | |
Comment Actions
Fortunately, the tilde-expanding function from libedit is nicely exported. Too bad this code is duplicated between sh and libedit.