Re: [Emacs-diffs] master 2291d9a: Fix python.el for Emacs 24, bump ELPA version to 0.26.1 (Bug#30633)

Re: [Emacs-diffs] master 2291d9a: Fix python.el for Emacs 24, bump ELPA version to 0.26.1 (Bug#30633)

[Stefan Monnier]

>     Since python.el is distributed via GNU ELPA, it should be functional
>     in earlier Emacs versions.  Also fix some compile warnings.

Thanks.

I have a few questions, tho:

> +  (unless (fboundp 'file-local-name)

Regarding this function, [ I understand you didn't write this code, so
the question is not necessarily for you ] I see it's used as follows:

         (file-name (expand-file-name (file-local-name file-name)))
         (temp-file-name (when temp-file-name
                           (expand-file-name
                            (file-local-name temp-file-name)))))

I don't understand how/why it makes sense to apply expand-file-name on
its return value.  Shouldn't this be

         (file-name (file-local-name (expand-file-name file-name)))
         (temp-file-name (when temp-file-name
                           (file-local-name
                            (expand-file-name temp-file-name)))))

?

> +                                  "
> +Overlapping strings detected (start=%d, last-end=%d)")

You can put a \ at the end of the first line to avoid inserting
a newline in the string's content.

> -  (set (make-local-variable 'prettify-symbols-alist)
> -       python--prettify-symbols-alist)
> +  (when (and (boundp 'prettify-symbols-alist)
> +             (boundp 'python--prettify-symbols-alist))
> +    (set (make-local-variable 'prettify-symbols-alist)
> +         python--prettify-symbols-alist))

Setting prettify-symbols-alist when it's not defined should be harmless.
So I'm curious what was the motivation behind the (boundp
'prettify-symbols-alist) test?
Could we replace it with a (defvar prettify-symbols-alist)?

[ As for python--prettify-symbols-alist, it's defined above, so it will
  always be 'boundp'.  ]


        Stefan

Re: [Emacs-diffs] master 2291d9a: Fix python.el for Emacs 24, bump ELPA version to 0.26.1 (Bug#30633)

[Noam Postavsky]

On Tue, Feb 27, 2018 at 9:03 PM, Stefan Monnier
<monnier@iro.umontreal.ca> wrote:

>> +                                  "
>> +Overlapping strings detected (start=%d, last-end=%d)")
>
> You can put a \ at the end of the first line to avoid inserting
> a newline in the string's content.

Oops. I meant to do that, but somehow the backslash disappeared on me
(a paredit boosted typo, I guess).

>> -  (set (make-local-variable 'prettify-symbols-alist)
>> -       python--prettify-symbols-alist)
>> +  (when (and (boundp 'prettify-symbols-alist)
>> +             (boundp 'python--prettify-symbols-alist))
>> +    (set (make-local-variable 'prettify-symbols-alist)
>> +         python--prettify-symbols-alist))
>
> Setting prettify-symbols-alist when it's not defined should be harmless.
> So I'm curious what was the motivation behind the (boundp
> 'prettify-symbols-alist) test?
> Could we replace it with a (defvar prettify-symbols-alist)?

Yeah, I was just going through the compiler warnings, so defvar could work too.

> [ As for python--prettify-symbols-alist, it's defined above, so it will
>   always be 'boundp'.  ]

Oh, I see, in Emacs 24, the compiler doesn't realize that
define-obsolete-variable-alias defines a variable, so it still warns
about it. Actually, we could just use the non-obsolete
python-prettify-symbols-alist name there, right?

Re: [Emacs-diffs] master 2291d9a: Fix python.el for Emacs 24, bump ELPA version to 0.26.1 (Bug#30633)

[Stefan Monnier]

> Oh, I see, in Emacs 24, the compiler doesn't realize that
> define-obsolete-variable-alias defines a variable, so it still warns
> about it. Actually, we could just use the non-obsolete
> python-prettify-symbols-alist name there, right?

Indeed, we should use the non-obsolete name (and I now see also that the
define-obsolete-variable-alias is placed after the defvar, whereas it
should always come *before* (so that, in case one of the two is already
set when we get to define-obsolete-variable-alias we can just "share"
that setting, whereas if we do it afterwards we end up having to choose
between two conflicting settings)).


        Stefan

Re: [Emacs-diffs] master 2291d9a: Fix python.el for Emacs 24, bump ELPA version to 0.26.1 (Bug#30633)

[Michael Albinus]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

>          (file-name (expand-file-name (file-local-name file-name)))
>          (temp-file-name (when temp-file-name
>                            (expand-file-name
>                             (file-local-name temp-file-name)))))
>
> I don't understand how/why it makes sense to apply expand-file-name on
> its return value.  Shouldn't this be
>
>          (file-name (file-local-name (expand-file-name file-name)))
>          (temp-file-name (when temp-file-name
>                            (file-local-name
>                             (expand-file-name temp-file-name)))))
>
> ?

Yes. If, for example, file-name is "/ssh:user@host:~/what/ever", the
former forms might fail, because "~/what/ever" could be expanded
differently on local and remote hosts.

>         Stefan

Best regards, Michael.

Re: [Emacs-diffs] master 2291d9a: Fix python.el for Emacs 24, bump ELPA version to 0.26.1 (Bug#30633)

[Noam Postavsky]

On Wed, Feb 28, 2018 at 2:55 AM, Michael Albinus <michael.albinus@gmx.de> wrote:
> Stefan Monnier <monnier@IRO.UMontreal.CA> writes:
>
>>          (file-name (expand-file-name (file-local-name file-name)))
>>          (temp-file-name (when temp-file-name
>>                            (expand-file-name
>>                             (file-local-name temp-file-name)))))
>>
>> I don't understand how/why it makes sense to apply expand-file-name on
>> its return value.  Shouldn't this be
>>
>>          (file-name (file-local-name (expand-file-name file-name)))
>>          (temp-file-name (when temp-file-name
>>                            (file-local-name
>>                             (expand-file-name temp-file-name)))))
>>
>> ?
>
> Yes. If, for example, file-name is "/ssh:user@host:~/what/ever", the
> former forms might fail, because "~/what/ever" could be expanded
> differently on local and remote hosts.

Thanks, I fixed this and the other issues Stefan pointed out.

[1: 4a09341921]: 2018-03-01 22:07:21 -0500
  Fix issues turned up by previous python.el change
  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=4a0934192176fb8e372170f5f028edcf0f8cbdc3