Subject: Re: bug#59804: shell-resync-dirs hangs in (t)csh

From: Eli Zaretskii <[email-masked]>
Date: Sun, 04 Dec 2022 21:15:27 +0200

> From: Nicolas Graner <[email-masked]>
> Cc: [email-masked]
> Date: Sun, 04 Dec 2022 19:41:24 +0100
> 
> > --- a/lisp/shell.el
> > +++ b/lisp/shell.el
> > @@ -1162,6 +1162,7 @@ shell-resync-dirs
> >           (dlsl nil)
> >           (pos 0)
> >           (ds nil))
> > +    (setq dls (string-trim-right dls "[ ]+"))
> >      ;; Split the dirlist into whitespace and non-whitespace chunks.
> >      ;; dlsl will be a reversed list of tokens.
> >      (while (string-match "\\(\\S-+\\|\\s-+\\)" dls pos)
> 
> It works in most situations, but not if a directory name actually ends
> with a space.
> Upon further investigation, I found there are two distinct issues here.
> 
> 1) When the dirs command prints "foo ", there is no way to tell if the
> directory is named "foo" and the shell (e.g. csh) added a trailing
> space, or the directory is named "foo " and the shell (e.g. bash)
> printed it unchanged. There may even be more than one trailing space.
> 
> The only clean solution I can think of is to introduce a new
> shell-dependent variable, similar to shell-dirstack-query, say
> shell-dirstack-query-suffix, which would be " " for csh and tcsh, and ""
> for most other shells. Your patch above would become:
> 
>   (setq dls (string-trim-right dls shell-dirstack-query-suffix))

Is it certain that csh/tcsh always add just one space?  If so, we could
simply remove that one space in the case of these two shells.

> The following patch fixes the bug by ensuring the loop terminates in all
> cases, but I am not sure it always does the right thing when no
> directory name is found. It should be reviewed by someone who
> understands the code better than me.

OK, thanks.  I think I'd prefer for now to fix just the trailing space part,
which AFAIU happens always, and leave the more exotic situations as a FIXME
comment, or maybe I will make the change you propose on the master branch.