Don't tab complete to parameters on other nodes#26
Don't tab complete to parameters on other nodes#26kivkiv12345 wants to merge 3 commits intomasterfrom
Conversation
Also inform the user when they haven't downloaded any parameters from the specified node
fadeto404
left a comment
fadeto404
left a comment
There was a problem hiding this comment.
Looks good, I like all improvements to auto completion! :D
A nice addition would be to print all parameters on the node if auto completing without a token (e.g. "get "/"get -n 130 ").
A passing thought is that you might have been able to save yourself some trouble by scanning the token for " -n %d" instead of rebuilding the arguments, but maybe this is more robust.
♫
|
|
||
| if (skip_prefix == NULL) { | ||
| fprintf(stderr, "%s() does not support the line/command: '%s'", __func__, slash->buffer); | ||
| assert(false); // Assert because this should be a compile-time error on part of the programmer, not the user. |
There was a problem hiding this comment.
assert is a run-time thing ;)
There was a problem hiding this comment.
I can't deny that. To clarify, I meant that this check should only ever fail if a compile-time change is made elsewhere in the program. So that's why this asserts, rather than returning an error.
I don't think this interpretation of asserts is exclusive to Python.
| } | ||
|
|
||
| /* Build args */ | ||
| int slash_build_args(char *args, char **argv, int *argc); |
There was a problem hiding this comment.
I'm not a fan of this way of declaring a function, this is not really friendly to future changes.
It would be best to add this function to a header so that we know this function is used in several places...
There was a problem hiding this comment.
I agree it's a bit nasty. It seems I've misplaced my TODO on line 110-112 (inclusive).
I did consider properly exposing slash_build_args() in a header, but I would rather keep it as internal functionality for now.
The reason we use it anyway is because slash->argc and slash->argv aren't properly initialized, before the tab-completer is called. So we would ideally fix those instead. I had some trouble though, as hinted by my TODO.
There was a problem hiding this comment.
In that case, add it to a "private" header (usually, one in the "src" folder as the ones in the "include" folder are definitely public).
There was a problem hiding this comment.
The problem is that slash_build_args() comes from lib/slash/, and this commit is for lib/param/, so I don't think a "private" header would be visible to us.
There was a problem hiding this comment.
You know what to do then... Promote "slash_build_args" to a public API in a libslash Pull Request...
src/param/param_slash.c
Outdated
| #define SLASH_ARG_MAX 16 /* Maximum number of arguments */ | ||
| char *argv[SLASH_ARG_MAX]; | ||
| int argc = 0; | ||
| char *args = strdup(slash->buffer + strlen(skip_prefix)); |
There was a problem hiding this comment.
I don't see a corresponding call to "free", am I missing something?
There was a problem hiding this comment.
Nope. It was I, who was missing something :)
I must've simply gotten too used to attribute((cleanup())) for my own good.
I've added a corresponding free() in the newest commit (8962b67)
3 participants
Also inform the user when they haven't downloaded any parameters from the specified node