cwm: remove ssh auto-completion

classic Classic list List threaded Threaded
12 messages Options
Reply | Threaded
Open this post in threaded view
|

cwm: remove ssh auto-completion

Okan Demirmen
Hi cwm users,

In converting this to use getline(3) over fgetln(3), I'm asking the question if
this (imho) mis-feature belongs in a window manager. I've hinted at wanting to
remove it in the past but there was opposition.  As a compromise, we could just
leave the 'ssh>' menu available, but it will not be pre-populated (nor
auto-completed) by what's in ~/.ssh/known_hosts; just type in the
host/ip/whatever at the menu prompt. cwm(1) does a very rudimentary pass at
parsing the known_hosts file, and there's no reason to hoist code from ssh(1)
just to get auto-completion in a window manager menu.

Below is the minimal code part in kbfunc_menu_ssh().

Preferably, I'd like to remove the whole ssh menu, but can live with the below
if it's useful to others.

Thoughts?

Thanks,
Okan

Index: kbfunc.c
===================================================================
RCS file: /home/open/cvs/xenocara/app/cwm/kbfunc.c,v
retrieving revision 1.145
diff -u -p -r1.145 kbfunc.c
--- kbfunc.c 9 May 2017 13:40:18 -0000 1.145
+++ kbfunc.c 6 Jul 2017 19:36:13 -0000
@@ -462,50 +462,16 @@ kbfunc_menu_ssh(void *ctx, struct cargs
  struct cmd_ctx *cmd;
  struct menu *mi;
  struct menu_q menuq;
- FILE *fp;
- char *buf, *lbuf, *p;
- char hostbuf[HOST_NAME_MAX+1];
  char path[PATH_MAX];
  int l;
- size_t len;
 
  TAILQ_FOREACH(cmd, &Conf.cmdq, entry) {
  if (strcmp(cmd->name, "term") == 0)
  break;
  }
- TAILQ_INIT(&menuq);
 
- if ((fp = fopen(Conf.known_hosts, "r")) == NULL) {
- warn("%s: %s", __func__, Conf.known_hosts);
- goto menu;
- }
+ TAILQ_INIT(&menuq);
 
- lbuf = NULL;
- while ((buf = fgetln(fp, &len))) {
- if (buf[len - 1] == '\n')
- buf[len - 1] = '\0';
- else {
- /* EOF without EOL, copy and add the NUL */
- lbuf = xmalloc(len + 1);
- (void)memcpy(lbuf, buf, len);
- lbuf[len] = '\0';
- buf = lbuf;
- }
- /* skip hashed hosts */
- if (strncmp(buf, HASH_MARKER, strlen(HASH_MARKER)) == 0)
- continue;
- for (p = buf; *p != ',' && *p != ' ' && p != buf + len; p++) {
- /* do nothing */
- }
- /* ignore badness */
- if (p - buf + 1 > sizeof(hostbuf))
- continue;
- (void)strlcpy(hostbuf, buf, p - buf + 1);
- menuq_add(&menuq, NULL, "%s", hostbuf);
- }
- free(lbuf);
- (void)fclose(fp);
-menu:
  if ((mi = menu_filter(sc, &menuq, "ssh", NULL, (CWM_MENU_DUMMY),
     search_match_text, search_print_text)) != NULL) {
  if (mi->text[0] == '\0')

Reply | Threaded
Open this post in threaded view
|

Re: cwm: remove ssh auto-completion

Bryan Steele-2
On Fri, Jul 07, 2017 at 12:44:35PM -0400, Okan Demirmen wrote:

> Hi cwm users,
>
> In converting this to use getline(3) over fgetln(3), I'm asking the question if
> this (imho) mis-feature belongs in a window manager. I've hinted at wanting to
> remove it in the past but there was opposition.  As a compromise, we could just
> leave the 'ssh>' menu available, but it will not be pre-populated (nor
> auto-completed) by what's in ~/.ssh/known_hosts; just type in the
> host/ip/whatever at the menu prompt. cwm(1) does a very rudimentary pass at
> parsing the known_hosts file, and there's no reason to hoist code from ssh(1)
> just to get auto-completion in a window manager menu.
>
> Below is the minimal code part in kbfunc_menu_ssh().
>
> Preferably, I'd like to remove the whole ssh menu, but can live with the below
> if it's useful to others.
>
> Thoughts?
>
> Thanks,
> Okan

Just my opinion, users can already set short aliases in their
.ssh/config files, so having to type in the host manually
shouldn't be too difficult.

I've never used this feature, I also typically don't expect auto
completion from every window manager UI context, unless for cmds
and paths.

-Bryan.

> Index: kbfunc.c
> ===================================================================
> RCS file: /home/open/cvs/xenocara/app/cwm/kbfunc.c,v
> retrieving revision 1.145
> diff -u -p -r1.145 kbfunc.c
> --- kbfunc.c 9 May 2017 13:40:18 -0000 1.145
> +++ kbfunc.c 6 Jul 2017 19:36:13 -0000
> @@ -462,50 +462,16 @@ kbfunc_menu_ssh(void *ctx, struct cargs
>   struct cmd_ctx *cmd;
>   struct menu *mi;
>   struct menu_q menuq;
> - FILE *fp;
> - char *buf, *lbuf, *p;
> - char hostbuf[HOST_NAME_MAX+1];
>   char path[PATH_MAX];
>   int l;
> - size_t len;
>  
>   TAILQ_FOREACH(cmd, &Conf.cmdq, entry) {
>   if (strcmp(cmd->name, "term") == 0)
>   break;
>   }
> - TAILQ_INIT(&menuq);
>  
> - if ((fp = fopen(Conf.known_hosts, "r")) == NULL) {
> - warn("%s: %s", __func__, Conf.known_hosts);
> - goto menu;
> - }
> + TAILQ_INIT(&menuq);
>  
> - lbuf = NULL;
> - while ((buf = fgetln(fp, &len))) {
> - if (buf[len - 1] == '\n')
> - buf[len - 1] = '\0';
> - else {
> - /* EOF without EOL, copy and add the NUL */
> - lbuf = xmalloc(len + 1);
> - (void)memcpy(lbuf, buf, len);
> - lbuf[len] = '\0';
> - buf = lbuf;
> - }
> - /* skip hashed hosts */
> - if (strncmp(buf, HASH_MARKER, strlen(HASH_MARKER)) == 0)
> - continue;
> - for (p = buf; *p != ',' && *p != ' ' && p != buf + len; p++) {
> - /* do nothing */
> - }
> - /* ignore badness */
> - if (p - buf + 1 > sizeof(hostbuf))
> - continue;
> - (void)strlcpy(hostbuf, buf, p - buf + 1);
> - menuq_add(&menuq, NULL, "%s", hostbuf);
> - }
> - free(lbuf);
> - (void)fclose(fp);
> -menu:
>   if ((mi = menu_filter(sc, &menuq, "ssh", NULL, (CWM_MENU_DUMMY),
>      search_match_text, search_print_text)) != NULL) {
>   if (mi->text[0] == '\0')

Reply | Threaded
Open this post in threaded view
|

Re: cwm: remove ssh auto-completion

Olivier Cherrier
In reply to this post by Okan Demirmen
On Fri, Jul 07, 2017 at 12:44:35PM -0400, [hidden email] wrote:

> Hi cwm users,
>
> In converting this to use getline(3) over fgetln(3), I'm asking the question if
> this (imho) mis-feature belongs in a window manager. I've hinted at wanting to
> remove it in the past but there was opposition.  As a compromise, we could just
> leave the 'ssh>' menu available, but it will not be pre-populated (nor
> auto-completed) by what's in ~/.ssh/known_hosts; just type in the
> host/ip/whatever at the menu prompt. cwm(1) does a very rudimentary pass at
> parsing the known_hosts file, and there's no reason to hoist code from ssh(1)
> just to get auto-completion in a window manager menu.
>
> Below is the minimal code part in kbfunc_menu_ssh().
>
> Preferably, I'd like to remove the whole ssh menu, but can live with the below
> if it's useful to others.
>
> Thoughts?

Hi Okan,

I use it alot and find it very light and useful.

Thanks,
Best
Olivier

Reply | Threaded
Open this post in threaded view
|

Re: cwm: remove ssh auto-completion

Martijn van Duren-5
In reply to this post by Okan Demirmen
As a sysadmin with access to over a couple of 100 machines I find this
feature extremely useful. It would require me quite a bit of extra
brainpower to remember the correct/full hostname of each and every one
of them, especially since I can start typing halfway through the
hostname.

So yeah, I think it'd would be a shame if this feature would disappear.

martijn@

On 07/07/17 18:44, Okan Demirmen wrote:

> Hi cwm users,
>
> In converting this to use getline(3) over fgetln(3), I'm asking the question if
> this (imho) mis-feature belongs in a window manager. I've hinted at wanting to
> remove it in the past but there was opposition.  As a compromise, we could just
> leave the 'ssh>' menu available, but it will not be pre-populated (nor
> auto-completed) by what's in ~/.ssh/known_hosts; just type in the
> host/ip/whatever at the menu prompt. cwm(1) does a very rudimentary pass at
> parsing the known_hosts file, and there's no reason to hoist code from ssh(1)
> just to get auto-completion in a window manager menu.
>
> Below is the minimal code part in kbfunc_menu_ssh().
>
> Preferably, I'd like to remove the whole ssh menu, but can live with the below
> if it's useful to others.
>
> Thoughts?
>
> Thanks,
> Okan
>
> Index: kbfunc.c
> ===================================================================
> RCS file: /home/open/cvs/xenocara/app/cwm/kbfunc.c,v
> retrieving revision 1.145
> diff -u -p -r1.145 kbfunc.c
> --- kbfunc.c 9 May 2017 13:40:18 -0000 1.145
> +++ kbfunc.c 6 Jul 2017 19:36:13 -0000
> @@ -462,50 +462,16 @@ kbfunc_menu_ssh(void *ctx, struct cargs
>   struct cmd_ctx *cmd;
>   struct menu *mi;
>   struct menu_q menuq;
> - FILE *fp;
> - char *buf, *lbuf, *p;
> - char hostbuf[HOST_NAME_MAX+1];
>   char path[PATH_MAX];
>   int l;
> - size_t len;
>  
>   TAILQ_FOREACH(cmd, &Conf.cmdq, entry) {
>   if (strcmp(cmd->name, "term") == 0)
>   break;
>   }
> - TAILQ_INIT(&menuq);
>  
> - if ((fp = fopen(Conf.known_hosts, "r")) == NULL) {
> - warn("%s: %s", __func__, Conf.known_hosts);
> - goto menu;
> - }
> + TAILQ_INIT(&menuq);
>  
> - lbuf = NULL;
> - while ((buf = fgetln(fp, &len))) {
> - if (buf[len - 1] == '\n')
> - buf[len - 1] = '\0';
> - else {
> - /* EOF without EOL, copy and add the NUL */
> - lbuf = xmalloc(len + 1);
> - (void)memcpy(lbuf, buf, len);
> - lbuf[len] = '\0';
> - buf = lbuf;
> - }
> - /* skip hashed hosts */
> - if (strncmp(buf, HASH_MARKER, strlen(HASH_MARKER)) == 0)
> - continue;
> - for (p = buf; *p != ',' && *p != ' ' && p != buf + len; p++) {
> - /* do nothing */
> - }
> - /* ignore badness */
> - if (p - buf + 1 > sizeof(hostbuf))
> - continue;
> - (void)strlcpy(hostbuf, buf, p - buf + 1);
> - menuq_add(&menuq, NULL, "%s", hostbuf);
> - }
> - free(lbuf);
> - (void)fclose(fp);
> -menu:
>   if ((mi = menu_filter(sc, &menuq, "ssh", NULL, (CWM_MENU_DUMMY),
>      search_match_text, search_print_text)) != NULL) {
>   if (mi->text[0] == '\0')
>

Reply | Threaded
Open this post in threaded view
|

Re: cwm: remove ssh auto-completion

Todd T. Fries-2
I'm going to echo Martijn's opinion below.

Nearly 95% of my ssh sessions to lots of systems start with alt-. followed
by a few keys typed to confirm the hostname is correct followed by enter.

Typing the full hostname or providing shortened versions in .ssh/config is
unwieldy and quite cumbersome otherwise.

I've come to use this feature as a very trusted time saver in my regular
day.

I'd say I use it on average 100 or more times per day.

What are our options to replace it with equivalent functionality should you
feel the need to march on without it?

Thanks,

Penned by Martijn van Duren on 20170710  0:01.54, we have:
| As a sysadmin with access to over a couple of 100 machines I find this
| feature extremely useful. It would require me quite a bit of extra
| brainpower to remember the correct/full hostname of each and every one
| of them, especially since I can start typing halfway through the
| hostname.
|
| So yeah, I think it'd would be a shame if this feature would disappear.
|
| martijn@
|
| On 07/07/17 18:44, Okan Demirmen wrote:
| > Hi cwm users,
| >
| > In converting this to use getline(3) over fgetln(3), I'm asking the question if
| > this (imho) mis-feature belongs in a window manager. I've hinted at wanting to
| > remove it in the past but there was opposition.  As a compromise, we could just
| > leave the 'ssh>' menu available, but it will not be pre-populated (nor
| > auto-completed) by what's in ~/.ssh/known_hosts; just type in the
| > host/ip/whatever at the menu prompt. cwm(1) does a very rudimentary pass at
| > parsing the known_hosts file, and there's no reason to hoist code from ssh(1)
| > just to get auto-completion in a window manager menu.
| >
| > Below is the minimal code part in kbfunc_menu_ssh().
| >
| > Preferably, I'd like to remove the whole ssh menu, but can live with the below
| > if it's useful to others.
| >
| > Thoughts?
| >
| > Thanks,
| > Okan
| >
| > Index: kbfunc.c
| > ===================================================================
| > RCS file: /home/open/cvs/xenocara/app/cwm/kbfunc.c,v
| > retrieving revision 1.145
| > diff -u -p -r1.145 kbfunc.c
| > --- kbfunc.c 9 May 2017 13:40:18 -0000 1.145
| > +++ kbfunc.c 6 Jul 2017 19:36:13 -0000
| > @@ -462,50 +462,16 @@ kbfunc_menu_ssh(void *ctx, struct cargs
| >   struct cmd_ctx *cmd;
| >   struct menu *mi;
| >   struct menu_q menuq;
| > - FILE *fp;
| > - char *buf, *lbuf, *p;
| > - char hostbuf[HOST_NAME_MAX+1];
| >   char path[PATH_MAX];
| >   int l;
| > - size_t len;
| >  
| >   TAILQ_FOREACH(cmd, &Conf.cmdq, entry) {
| >   if (strcmp(cmd->name, "term") == 0)
| >   break;
| >   }
| > - TAILQ_INIT(&menuq);
| >  
| > - if ((fp = fopen(Conf.known_hosts, "r")) == NULL) {
| > - warn("%s: %s", __func__, Conf.known_hosts);
| > - goto menu;
| > - }
| > + TAILQ_INIT(&menuq);
| >  
| > - lbuf = NULL;
| > - while ((buf = fgetln(fp, &len))) {
| > - if (buf[len - 1] == '\n')
| > - buf[len - 1] = '\0';
| > - else {
| > - /* EOF without EOL, copy and add the NUL */
| > - lbuf = xmalloc(len + 1);
| > - (void)memcpy(lbuf, buf, len);
| > - lbuf[len] = '\0';
| > - buf = lbuf;
| > - }
| > - /* skip hashed hosts */
| > - if (strncmp(buf, HASH_MARKER, strlen(HASH_MARKER)) == 0)
| > - continue;
| > - for (p = buf; *p != ',' && *p != ' ' && p != buf + len; p++) {
| > - /* do nothing */
| > - }
| > - /* ignore badness */
| > - if (p - buf + 1 > sizeof(hostbuf))
| > - continue;
| > - (void)strlcpy(hostbuf, buf, p - buf + 1);
| > - menuq_add(&menuq, NULL, "%s", hostbuf);
| > - }
| > - free(lbuf);
| > - (void)fclose(fp);
| > -menu:
| >   if ((mi = menu_filter(sc, &menuq, "ssh", NULL, (CWM_MENU_DUMMY),
| >      search_match_text, search_print_text)) != NULL) {
| >   if (mi->text[0] == '\0')
| >

--
Todd T. Fries . http://todd.fries.net/pgp.txt . @unix2mars . github:toddfries

Reply | Threaded
Open this post in threaded view
|

Re: cwm: remove ssh auto-completion

Stuart Henderson
On 2017/07/10 00:56, Todd T. Fries wrote:
> What are our options to replace it with equivalent functionality should you
> feel the need to march on without it?

ssh `cut -d' ' -f1 .ssh/known_hosts | dmenu`

Reply | Threaded
Open this post in threaded view
|

Re: cwm: remove ssh auto-completion

Bryan Steele-2
On Mon, Jul 10, 2017 at 07:59:15AM +0100, Stuart Henderson wrote:
> On 2017/07/10 00:56, Todd T. Fries wrote:
> > What are our options to replace it with equivalent functionality should you
> > feel the need to march on without it?
>
> ssh `cut -d' ' -f1 .ssh/known_hosts | dmenu`

Instead of relying on the window manager, why not add it to your
shell?

https://deftly.net/posts/2017-05-01-openbsd-ksh-tab-complete.html

  set -A complete_ssh $(awk '!/\*/ && /^Host /{print $2}' ~/.ssh/config)

I'm sure someone can come up with a cool one liner for parsing
known_hosts files.

-Bryan.

Reply | Threaded
Open this post in threaded view
|

Re: cwm: remove ssh auto-completion

Ingo Schwarze
Hi,

Bryan Steele wrote on Mon, Jul 10, 2017 at 08:21:19AM -0400:

> Instead of relying on the window manager, why not add it to your
> shell?
>
> https://deftly.net/posts/2017-05-01-openbsd-ksh-tab-complete.html
>
>   set -A complete_ssh $(awk '!/\*/ && /^Host /{print $2}' ~/.ssh/config)
>
> I'm sure someone can come up with a cool one liner for parsing
> known_hosts files.

YIKES, what a horrible idea.

I think the support for complete_* arrays ought to be summarily
deleted from emacs.c.  It is excessive complexity and creeping
featurism and has no place in a program as sensitve as the shell.

Yours,
  Ingo

Reply | Threaded
Open this post in threaded view
|

Re: cwm: remove ssh auto-completion

Bryan Steele-2
On Mon, Jul 10, 2017 at 03:22:48PM +0200, Ingo Schwarze wrote:

> Hi,
>
> Bryan Steele wrote on Mon, Jul 10, 2017 at 08:21:19AM -0400:
>
> > Instead of relying on the window manager, why not add it to your
> > shell?
> >
> > https://deftly.net/posts/2017-05-01-openbsd-ksh-tab-complete.html
> >
> >   set -A complete_ssh $(awk '!/\*/ && /^Host /{print $2}' ~/.ssh/config)
> >
> > I'm sure someone can come up with a cool one liner for parsing
> > known_hosts files.
>
> YIKES, what a horrible idea.
>
> I think the support for complete_* arrays ought to be summarily
> deleted from emacs.c.  It is excessive complexity and creeping
> featurism and has no place in a program as sensitve as the shell.
>
> Yours,
>   Ingo

?

This was a feature added last year by nicm@, not touching emacs.c
at all..

http://marc.info/?l=openbsd-cvs&m=147300974112400&w=2

http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/bin/ksh/edit.c.diff?r1=1.57&r2=1.54

-Bryan.

Reply | Threaded
Open this post in threaded view
|

Re: cwm: remove ssh auto-completion

Ingo Schwarze
Hi,

Bryan Steele wrote on Mon, Jul 10, 2017 at 09:38:32AM -0400:

> This was a feature added last year by nicm@, not touching emacs.c
> at all..

edit.c is a helper file containing common utilities for emacs.c
and vi.c.  So it is also misdocumented.  It is only documented
for emacs mode, but vi mode does it, too.

> http://marc.info/?l=openbsd-cvs&m=147300974112400&w=2
> http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/bin/ksh/edit.c.diff?r1=1.57&r2=1.54

I must have missed it.
I probably would have opposed it if i had noticed it.

Yours,
  Ingo

Reply | Threaded
Open this post in threaded view
|

Re: cwm: remove ssh auto-completion

Landry Breuil-5
On Mon, Jul 10, 2017 at 04:04:47PM +0200, Ingo Schwarze wrote:

> Hi,
>
> Bryan Steele wrote on Mon, Jul 10, 2017 at 09:38:32AM -0400:
>
> > This was a feature added last year by nicm@, not touching emacs.c
> > at all..
>
> edit.c is a helper file containing common utilities for emacs.c
> and vi.c.  So it is also misdocumented.  It is only documented
> for emacs mode, but vi mode does it, too.
>
> > http://marc.info/?l=openbsd-cvs&m=147300974112400&w=2
> > http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/bin/ksh/edit.c.diff?r1=1.57&r2=1.54
>
> I must have missed it.
> I probably would have opposed it if i had noticed it.

Ingo,

i agree that sometimes you need to keep stuff clean and readable and
etc, but if you are at a point that your shell doesn't offer you
'basic' features (present in almost every other shell) that *improves*
your productivity and you need to use another shell for that, then
you've lost.

I, for one, welcome the ability to easily define such completions,
especially for ssh hosts when you have to manage dozens of them.

Landry

Reply | Threaded
Open this post in threaded view
|

Re: cwm: remove ssh auto-completion

Todd T. Fries-2
In reply to this post by Ingo Schwarze
Wow, I missed the completion bits also!

I like this better than the cwm menu or dmenu, so I'll be using this
from now on instead of the window manager.

Here's my cool 3 liners to nab all them hosts ;-)

set -A complete_ssh $(
        (awk '!/\*/ && /^Host /{print $2}' ~/.ssh/config;
        awk '{sub(",.*$","",$1);print $1}' ~/.ssh/known_hosts)|sort -u)

Thanks!

Penned by Ingo Schwarze on 20170710  8:22.48, we have:
| Hi,
|
| Bryan Steele wrote on Mon, Jul 10, 2017 at 08:21:19AM -0400:
|
| > Instead of relying on the window manager, why not add it to your
| > shell?
| >
| > https://deftly.net/posts/2017-05-01-openbsd-ksh-tab-complete.html
| >
| >   set -A complete_ssh $(awk '!/\*/ && /^Host /{print $2}' ~/.ssh/config)
| >
| > I'm sure someone can come up with a cool one liner for parsing
| > known_hosts files.
|
| YIKES, what a horrible idea.
|
| I think the support for complete_* arrays ought to be summarily
| deleted from emacs.c.  It is excessive complexity and creeping
| featurism and has no place in a program as sensitve as the shell.
|
| Yours,
|   Ingo

--
Todd T. Fries . http://todd.fries.net/pgp.txt . @unix2mars . github:toddfries