GNU bug report logs - #79036
[PATCH] Fix pdb tracking for remote filenames

Previous Next

Full log

Message #20 received at 79036 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: liuhui1610 <at> gmail.com, Michael Albinus <michael.albinus <at> gmx.de>,
 kobarity <kobarity <at> gmail.com>
Cc: 79036 <at> debbugs.gnu.org
Subject: Re: bug#79036: [PATCH] Fix pdb tracking for remote filenames
Date: Thu, 17 Jul 2025 11:45:55 +0300

> From: Michael Albinus <michael.albinus <at> gmx.de>
> Cc: liuhui1610 <at> gmail.com,  79036 <at> debbugs.gnu.org
> Date: Thu, 17 Jul 2025 10:18:41 +0200
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> Hi Eli,
> 
> >> >> diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
> >> >> index f4f0518dbfd..59977fcb49f 100644
> >> >> --- a/lisp/progmodes/python.el
> >> >> +++ b/lisp/progmodes/python.el
> >> >> @@ -5079,8 +5079,10 @@ python-pdbtrack-set-tracked-buffer
> >> >>    "Set the buffer for FILE-NAME as the tracked buffer.
> >> >>  Internally it uses the `python-pdbtrack-tracked-buffer' variable.
> >> >>  Returns the tracked buffer."
> >> >> -  (let* ((file-name-prospect (concat (file-remote-p default-directory)
> >> >> -                              file-name))
> >> >> +  (let* ((file-name-prospect (if (file-remote-p file-name)
> >> >> +                                 file-name
> >> >> +                               (concat (file-remote-p default-directory)
> >> >> +                                       file-name)))
> >> >>           (file-buffer (get-file-buffer file-name-prospect)))
> >> >>      (unless file-buffer
> >> >>        (cond
> >> >
> >> > Shouldn't this code use 'expand-file-name' instead?  Using 'concat' to
> >> > construct file names is a bug waiting to happen, IME.
> >> 
> >> `expand-file-name' wouldn't work if file-name is an absolute file name.
> >
> > I guess I'm missing something: doesn't this code want to produce an
> > absolute file name?  If not, what does it try to do?
> >
> >> `file-remote-p' is designed to cooperate with `concat'. It mentions it
> >> in its docstring.
> >
> > So you agree with the patch?  It looks strange to me that we should
> > use file-remote-p in the branch where it is known that FILE-NAME is
> > not a remote file name.
> 
> I didn't check the code, so I don't know whether file-name is guaranteed
> to be a relative file name. The docstring doesn't tell about this promise.
> 
> I just checked the file-remote-p construct, used with concat.

Thanks.

So now, Liu Hui, please explain the problem in more detail.

I stared at the code for some time, and I don't understand why it goes
to such great lengths here:

  (let* ((file-name-prospect (concat (file-remote-p default-directory)
                              file-name))
         (file-buffer (get-file-buffer file-name-prospect)))
    (unless file-buffer
      (cond
       ((file-exists-p file-name-prospect)
        (setq file-buffer (find-file-noselect file-name-prospect)))
       ((and (not (equal file-name file-name-prospect))
             (file-exists-p file-name))
        ;; Fallback to a locally available copy of the file.
        (setq file-buffer (find-file-noselect file-name-prospect))))

Why not just call find-file-noselect with FILE-NAME as its argument?
The function find-file-noselect will itself check if there's already a
buffer visiting that file, so it's unnecessary to do that "by hand"
here.  And I don't see the reason for that 'concat' dance, either,
since find-file-noselect again does everything that needs to be done
internally.  So what am I missing here?

kobarity, any comments or suggestions?

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.