I've noticed that "get" and "put" sftp commands offer correct completion for the first path (remote/local respectively), but incorrect for the second path (again, remote/local respectively). By tweaking the code a bit the completer can offer the right path for both arguments. This is a v2 of: https://marc.info/?l=openssh-unix-dev&m=164226681707781&w=2 diff to v1: - Rebase onto current master as the old version no longer applies cleanly. Michal Privoznik (2): sftp: Don't attempt to complete arguments for non-existent commands sftp: Be a bit more clever about completions sftp.c | 113 ++++++++++++++++++++++++++++++++++----------------------- 1 file changed, 67 insertions(+), 46 deletions(-) -- 2.35.1
Michal Privoznik
2022-Sep-12 14:17 UTC
[PATCH v2 1/2] sftp: Don't attempt to complete arguments for non-existent commands
If user entered a non-existent command (e.g. because they made a typo) there is no point in trying to complete its arguments. Skip calling complete_match() if that's the case. Signed-off-by: Michal Privoznik <mprivozn at redhat.com> --- sftp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sftp.c b/sftp.c index 939b8dc0..70e8d344 100644 --- a/sftp.c +++ b/sftp.c @@ -2143,7 +2143,7 @@ complete(EditLine *el, int ch) if (carg > 1 && line[cursor-1] != ' ') filematch = argv[carg - 1]; - if (remote != 0 && + if ((remote == REMOTE || remote == LOCAL) && complete_match(el, complete_ctx->conn, *complete_ctx->remote_pathp, filematch, remote, carg == argc, quote, terminated) != 0) -- 2.35.1
Michal Privoznik
2022-Sep-12 14:17 UTC
[PATCH v2 2/2] sftp: Be a bit more clever about completions
There are commands (e.g. "get" or "put") that accept two
arguments, a local path and a remote path. However, the way
current completion is written doesn't take this distinction into
account and always completes remote or local paths.
By expanding CMD struct and "cmds" array this distinction can be
reflected and with small adjustment to completer code the correct
path can be completed.
Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
---
sftp.c | 111 ++++++++++++++++++++++++++++++++++-----------------------
1 file changed, 66 insertions(+), 45 deletions(-)
diff --git a/sftp.c b/sftp.c
index 70e8d344..9efe43c7 100644
--- a/sftp.c
+++ b/sftp.c
@@ -166,7 +166,8 @@ enum sftp_command {
struct CMD {
const char *c;
const int n;
- const int t;
+ const int t; /* Type of completion for the first argument */
+ const int t2; /* Type of completion for the optional second argument */
};
/* Type of completion */
@@ -175,44 +176,44 @@ struct CMD {
#define LOCAL 2
static const struct CMD cmds[] = {
- { "bye", I_QUIT, NOARGS },
- { "cd", I_CHDIR, REMOTE },
- { "chdir", I_CHDIR, REMOTE },
- { "chgrp", I_CHGRP, REMOTE },
- { "chmod", I_CHMOD, REMOTE },
- { "chown", I_CHOWN, REMOTE },
- { "copy", I_COPY, REMOTE },
- { "cp", I_COPY, REMOTE },
- { "df", I_DF, REMOTE },
- { "dir", I_LS, REMOTE },
- { "exit", I_QUIT, NOARGS },
- { "get", I_GET, REMOTE },
- { "help", I_HELP, NOARGS },
- { "lcd", I_LCHDIR, LOCAL },
- { "lchdir", I_LCHDIR, LOCAL },
- { "lls", I_LLS, LOCAL },
- { "lmkdir", I_LMKDIR, LOCAL },
- { "ln", I_LINK, REMOTE },
- { "lpwd", I_LPWD, LOCAL },
- { "ls", I_LS, REMOTE },
- { "lumask", I_LUMASK, NOARGS },
- { "mkdir", I_MKDIR, REMOTE },
- { "mget", I_GET, REMOTE },
- { "mput", I_PUT, LOCAL },
- { "progress", I_PROGRESS, NOARGS },
- { "put", I_PUT, LOCAL },
- { "pwd", I_PWD, REMOTE },
- { "quit", I_QUIT, NOARGS },
- { "reget", I_REGET, REMOTE },
- { "rename", I_RENAME, REMOTE },
- { "reput", I_REPUT, LOCAL },
- { "rm", I_RM, REMOTE },
- { "rmdir", I_RMDIR, REMOTE },
- { "symlink", I_SYMLINK, REMOTE },
- { "version", I_VERSION, NOARGS },
- { "!", I_SHELL, NOARGS },
- { "?", I_HELP, NOARGS },
- { NULL, -1, -1 }
+ { "bye", I_QUIT, NOARGS, NOARGS },
+ { "cd", I_CHDIR, REMOTE, NOARGS },
+ { "chdir", I_CHDIR, REMOTE, NOARGS },
+ { "chgrp", I_CHGRP, REMOTE, NOARGS },
+ { "chmod", I_CHMOD, REMOTE, NOARGS },
+ { "chown", I_CHOWN, REMOTE, NOARGS },
+ { "copy", I_COPY, REMOTE, LOCAL },
+ { "cp", I_COPY, REMOTE, LOCAL },
+ { "df", I_DF, REMOTE, NOARGS },
+ { "dir", I_LS, REMOTE, NOARGS },
+ { "exit", I_QUIT, NOARGS, NOARGS },
+ { "get", I_GET, REMOTE, LOCAL },
+ { "help", I_HELP, NOARGS, NOARGS },
+ { "lcd", I_LCHDIR, LOCAL, NOARGS },
+ { "lchdir", I_LCHDIR, LOCAL, NOARGS },
+ { "lls", I_LLS, LOCAL, NOARGS },
+ { "lmkdir", I_LMKDIR, LOCAL, NOARGS },
+ { "ln", I_LINK, REMOTE, REMOTE },
+ { "lpwd", I_LPWD, LOCAL, NOARGS },
+ { "ls", I_LS, REMOTE, NOARGS },
+ { "lumask", I_LUMASK, NOARGS, NOARGS },
+ { "mkdir", I_MKDIR, REMOTE, NOARGS },
+ { "mget", I_GET, REMOTE, LOCAL },
+ { "mput", I_PUT, LOCAL, REMOTE },
+ { "progress", I_PROGRESS, NOARGS, NOARGS },
+ { "put", I_PUT, LOCAL, REMOTE },
+ { "pwd", I_PWD, REMOTE, NOARGS },
+ { "quit", I_QUIT, NOARGS, NOARGS },
+ { "reget", I_REGET, REMOTE, LOCAL },
+ { "rename", I_RENAME, REMOTE, REMOTE },
+ { "reput", I_REPUT, LOCAL, REMOTE },
+ { "rm", I_RM, REMOTE, NOARGS },
+ { "rmdir", I_RMDIR, REMOTE, NOARGS },
+ { "symlink", I_SYMLINK, REMOTE, REMOTE },
+ { "version", I_VERSION, NOARGS, NOARGS },
+ { "!", I_SHELL, NOARGS, NOARGS },
+ { "?", I_HELP, NOARGS, NOARGS },
+ { NULL, -1, -1, -1 }
};
/* ARGSUSED */
@@ -1945,19 +1946,25 @@ complete_cmd_parse(EditLine *el, char *cmd, int lastarg,
char quote,
}
/*
- * Determine whether a particular sftp command's arguments (if any)
- * represent local or remote files.
+ * Determine whether a particular sftp command's arguments (if any)
represent
+ * local or remote files. The "cmdarg" argument specifies the actual
argument
+ * and accepts values 1 or 2.
*/
static int
-complete_is_remote(char *cmd) {
+complete_is_remote(char *cmd, int cmdarg) {
int i;
if (cmd == NULL)
return -1;
for (i = 0; cmds[i].c; i++) {
- if (!strncasecmp(cmd, cmds[i].c, strlen(cmds[i].c)))
- return cmds[i].t;
+ if (!strncasecmp(cmd, cmds[i].c, strlen(cmds[i].c))) {
+ if (cmdarg == 1)
+ return cmds[i].t;
+ else if (cmdarg == 2)
+ return cmds[i].t2;
+ break;
+ }
}
return -1;
@@ -2137,12 +2144,26 @@ complete(EditLine *el, int ch)
ret = CC_REDISPLAY;
} else if (carg >= 1) {
/* Handle file parsing */
- int remote = complete_is_remote(argv[0]);
+ int remote = 0;
+ int i = 0, cmdarg = 0;
char *filematch = NULL;
if (carg > 1 && line[cursor-1] != ' ')
filematch = argv[carg - 1];
+ for (i = 1; i < carg; i++) {
+ /* Skip flags */
+ if (argv[i][0] != '-')
+ cmdarg++;
+ }
+
+ /* If previous argument is complete, then offer completion on the next
+ * one. */
+ if (line[cursor - 1] == ' ')
+ cmdarg++;
+
+ remote = complete_is_remote(argv[0], cmdarg);
+
if ((remote == REMOTE || remote == LOCAL) &&
complete_match(el, complete_ctx->conn,
*complete_ctx->remote_pathp, filematch,
--
2.35.1
On Mon, 12 Sep 2022, Michal Privoznik wrote:> I've noticed that "get" and "put" sftp commands offer correct completion > for the first path (remote/local respectively), but incorrect for the > second path (again, remote/local respectively). By tweaking the code a > bit the completer can offer the right path for both arguments.Thanks! Both of these patches have been applied. -d