emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v3] Stop highlighting Python2 keywords.


From: Konstantin Kharlamov
Subject: Re: [PATCH v3] Stop highlighting Python2 keywords.
Date: Thu, 30 Jan 2025 22:00:04 +0300
User-agent: Evolution 3.54.3

On Fri, 2025-01-31 at 00:15 +0900, kobarity wrote:
> Konstantin Kharlamov wrote:
> > 
> > On Wed, 2025-01-29 at 19:56 -0600, Stefan Kangas wrote:
> > > Konstantin Kharlamov <Hi-Angel@yandex.ru> writes:
> > > 
> > > > On Wed, 2025-01-29 at 23:26 +0900, kobarity wrote:
> > > > > Konstantin Kharlamov wrote:
> > > > > > So, it's been a few days, nobody seems against, so I'm
> > > > > > attaching a
> > > > > > patch. Please review 😊
> > > > > 
> > > > > I think it should be announced in etc/NEWS.
> > > > 
> > > > Thank you, done!
> > > > From ffda4e954d157b544453b388458f1ec414d17abd Mon Sep 17
> > > > 00:00:00
> > > > 2001
> > > > From: Konstantin Kharlamov <Hi-Angel@yandex.ru>
> > > > Date: Wed, 29 Jan 2025 08:35:31 +0300
> > > > Subject: [PATCH v2] Remove Python2 "file" from the list of
> > > > keywords
> > > > 
> > > > `file` was a built-in type in Python2, removed in Python3. 
> > > > Nowadays,
> > > > word `file` is a frequent identifier name that people use in
> > > > short
> > > > file-related code sections.  Having it highlighted looks odd in
> > > > a
> > > > code.  As a prior art, Vim and VS Code don't currently
> > > > highlight
> > > > `file` specially.
> > > > 
> > > > This was recently discussed on emacs-devel with the conclusion
> > > > it
> > > > is
> > > > okay to remove `file` from the list of keywords.
> > > > 
> > > > * etc/NEWS: Announce "file" keyword removal.
> > > > * lisp/progmodes/python.el (python-font-lock-keywords-level-2):
> > > > Remove
> > > > "file" from the list.
> > > > ---
> > > >  etc/NEWS                 | 6 ++++++
> > > >  lisp/progmodes/python.el | 3 +--
> > > >  2 files changed, 7 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/etc/NEWS b/etc/NEWS
> > > > index f7e9f283709..ca16079ad64 100644
> > > > --- a/etc/NEWS
> > > > +++ b/etc/NEWS
> > > > @@ -807,6 +807,12 @@ exist.  If "python" points to Python 2 on
> > > > your
> > > > system, you now have to
> > > >  customize these variables to "python3" if you want to use
> > > > Python 3
> > > >  instead.
> > > > 
> > > > +---
> > > > +*** Remove "file" from the list of keywords.
> > > > +In Python 3 "file" has no special meaning, and in modern code
> > > > the
> > > > word
> > > > +is frequently used as a variable name.  To make sure highlight
> > > > in
> > > > such
> > > > +code doesn't look off, we no longer highlight "file".
> > > > +
> 
> Thanks.  If possible, it would be good to write this so that it is
> clear that this change is 'python-mode' specific and does not pertain
> to 'python-ts-mode'.
> 
> > > >  ---
> > > >  *** Support of 'electric-layout-mode' added.
> > > > 
> > > > diff --git a/lisp/progmodes/python.el
> > > > b/lisp/progmodes/python.el
> > > > index c00de2d6a8d..63798687928 100644
> > > > --- a/lisp/progmodes/python.el
> > > > +++ b/lisp/progmodes/python.el
> > > > @@ -715,9 +715,8 @@ python-font-lock-keywords-level-2
> > > >             "staticmethod" "str" "sum" "super" "tuple" "type"
> > > > "vars" "zip"
> > > >             "__import__"
> > > >             ;; Python 2:
> > > > -           "basestring" "cmp" "execfile" "file" "long"
> > > > "raw_input"
> > > > "reduce"
> > > > +           "basestring" "cmp" "execfile" "intern" "long"
> > > > "raw_input" "reduce"
> > > >             "reload" "unichr" "unicode" "xrange" "apply"
> > > > "buffer"
> > > > "coerce"
> > > > -           "intern"
> > > 
> > > I understand that "file" probably the top most common one, but,
> > > looking
> > > at the above list of builtins, many of the remaining ones should
> > > be
> > > attractive names to use as well:
> > > 
> > >     apply
> > >     basestring
> > >     buffer
> > >     cmp
> > >     coerce
> > >     execfile
> > >     file
> > >     intern
> > >     long
> > >     raw_input
> > >     reduce
> > >     reload
> > >     unichr
> > >     unicode
> > >     xrange
> > 
> > Not that I'm against, but I'd point out that to me basestring,
> > coerce,
> > execfile, intern, long, raw_input, reload, unichr, unicode, xrange
> > look
> > like highly unlikely variable names. "long" as I mentioned above
> > would
> > typically be part of a larger word. "buffer" I also don't recall
> > people
> > to write ever in full (people are more likely to type "buf").
> 
> I'm not against dropping all the Python 2 builtins either.
> 
> > And `reduce` wasn't really removed from Python3 (it's just not in
> > the
> > visibility scope by default), so it seems odd to remove given that
> > we
> > still highlight `all` and `any`.
> 
> Do you mean that we can import as follows?
> 
> #+begin_src python
> from functools import reduce
> #+end_src
> 
> I don't think it is a builtin.  There needs to be some criteria.
> 
> It might be nice to have a `defcustom' for the list of words to
> highlight as builtins, but it would be a bit complicated to introduce
> while maintaining compatibility with the current
> `python-font-lock-keywords-level-2',
> `python-font-lock-keywords-maximum-decoration', and
> `python-font-lock-keywords'.
> 
> > > If we're anyways making changes here, maybe we should just take
> > > the
> > > plunge and drop all of the Python 2 builtins?  It should be
> > > relatively
> > > harmless for Python 2 users, I think, and it's easier to explain
> > > what
> > > we're doing in etc/NEWS.
> 
> I don't have a strong preference for either, but I agree that it is
> easier to explain if all of the Python 2 builtins are dropped.

Okay, if you folks want, let's remove them all 😊 I don't have a
preference.

Attaching a patch, please see if it's okay. I'd like to point out the
code still has a `StandardError` Python 2 exception, should we drop it
as well then? I was about to do that, but then I realized this one
appears also in the python-ts-mode declared in the same file. I'm just
unsure how to declare it in the news then (like, is it "we remove all
python2 keywords from python-mode and StandardError from python-ts-
mode"?), and if it's even desirable?

Attachment: 1.patch
Description: Text Data


reply via email to

[Prev in Thread] Current Thread [Next in Thread]