unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* cperl-mode: Eliminating references to obsolete packages
@ 2020-09-23 18:37 Harald Jörg
  2020-09-23 19:16 ` Stefan Monnier
  0 siblings, 1 reply; 8+ messages in thread
From: Harald Jörg @ 2020-09-23 18:37 UTC (permalink / raw)
  To: emacs-devel

Hello Emacs,

My history with Emacs development is rather short, so I'd like to
check some assumptions. cperl-mode.el has quite some conditionals
checking for features of which I assume that they are now either
completely dead, or always available:

 - (featurep 'choose-color): choose-color.el used to be downloadable
   from Ilya Zakharevich's web page in the 20th century, but it is
   gone now.  I suggest it is safe to assume the value is always nil.

 - (featurep 'font-lock-extra): I failed to find any reference where
   this came from and suggest also to assume the value is always nil.

 - (featurep 'mode-compile): It seems that mode-compile.el was never
   part of GNU Emacs, and it seems to be abandoned by now.  I suggest
   it should be eliminated.

 - (boundp 'font-lock-constant-face): - also for other faces.  I guess
   it is safe to assume this is always true, and the current list of
   available faces as visible in the info page "Faces for Font Lock"
   hasn't changed (much) since Emacs 26.  I'd be grateful for a
   pointer to recent changes in this face list!

I'd like to prepare a patch which removes all conditionals querying
these functions, the associate code branches, and related
customization variables.  But before I do so, I'd like to check with
people who have been contributing to Emacs longer than I have.

Are there any opinions or objections?
--
Cheers,
haj



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: cperl-mode: Eliminating references to obsolete packages
  2020-09-23 18:37 cperl-mode: Eliminating references to obsolete packages Harald Jörg
@ 2020-09-23 19:16 ` Stefan Monnier
  2020-09-23 19:38   ` Harald Jörg
  0 siblings, 1 reply; 8+ messages in thread
From: Stefan Monnier @ 2020-09-23 19:16 UTC (permalink / raw)
  To: Harald Jörg; +Cc: emacs-devel

> I'd like to prepare a patch which removes all conditionals querying
> these functions, the associate code branches, and related
> customization variables.

Sounds good!
In the (distant) past I tended to try and keep the code unchanged to
minimize conflict with Ilya's upstream version, but AFAIK nowadays there
is no "upstream version" any more, so we can clean it up.  We may want
to keep the compatible with some older Emacsen (like Emacs-25), e.g. if
we want to distribute the code via GNU ELPA.


        Stefan




^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: cperl-mode: Eliminating references to obsolete packages
  2020-09-23 19:16 ` Stefan Monnier
@ 2020-09-23 19:38   ` Harald Jörg
  2020-09-23 19:50     ` Stefan Monnier
  0 siblings, 1 reply; 8+ messages in thread
From: Harald Jörg @ 2020-09-23 19:38 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

Stefan Monnier writes

>> I'd like to prepare a patch which removes all conditionals querying
>> these functions, the associate code branches, and related
>> customization variables.
> 
> Sounds good!
> In the (distant) past I tended to try and keep the code unchanged to
> minimize conflict with Ilya's upstream version, but AFAIK nowadays there
> is no "upstream version" any more, so we can clean it up.  We may want
> to keep the compatible with some older Emacsen (like Emacs-25), e.g. if
> we want to distribute the code via GNU ELPA.

I guess Ilya has retired?  Should I try to contact him?

I want to keep it compatible with at least Emacs 26, since this is the
version currently available with "stable" Linux distributions, and
this is the version used by Perlers who have shown interest or given
feedback so far.  As of now, I have found two commits which violate
that, one of which I consider fairly irrelevant (timing font-locking:
This was an issue 20 years ago, when computers where a bit slower).

I am unsure about the procedure: Should the backwards-compatible
version live in a separate branch on Emacs?  In the ELPA repository?
Or should incompatible changes just be avoided in the master branch?
There's still a lot to be done to modernize cperl-mode.el, and
manually keeping two versions in synch seems ugly.

Right now, I was lazy enough to create a branch in my own GitHub
repository which follows the master branch in the Emacs repository,
but eliminated one change so far which broke Emacs 26 compatibility:
The function format-prompt is new in Emacs 28.
-- 
Cheers,
haj



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: cperl-mode: Eliminating references to obsolete packages
  2020-09-23 19:38   ` Harald Jörg
@ 2020-09-23 19:50     ` Stefan Monnier
  2020-09-24 15:34       ` Harald Jörg
  0 siblings, 1 reply; 8+ messages in thread
From: Stefan Monnier @ 2020-09-23 19:50 UTC (permalink / raw)
  To: Harald Jörg; +Cc: emacs-devel

> I guess Ilya has retired?

I assume he stopped using Emacs, but I have no idea, really.

> Should I try to contact him?

I don't see any need for it, but feel free to try.

> I want to keep it compatible with at least Emacs 26, since this is the

Sounds good.

> feedback so far.  As of now, I have found two commits which violate
> that, one of which I consider fairly irrelevant (timing font-locking:
> This was an issue 20 years ago, when computers where a bit slower).

It's usually pretty easy to tweak changes so they don't break
backward compatibility.

> I am unsure about the procedure: Should the backwards-compatible
> version live in a separate branch on Emacs?

I think we don't need a separate branch.  E.g. for format-prompt we can
use a patch like the one below.  If you can't think of a simple way to
preserve backward compatibility in a specific situation, feel free to
ask for help,


        Stefan


diff --git a/lisp/progmodes/cperl-mode.el b/lisp/progmodes/cperl-mode.el
index 468ffc949a..57bee5c4d7 100644
--- a/lisp/progmodes/cperl-mode.el
+++ b/lisp/progmodes/cperl-mode.el
@@ -6324,6 +6324,12 @@ cperl-word-at-point
 		       (get major-mode 'find-tag-default-function)
 		       'find-tag-default))))))
 
+(defalias 'cperl--format-prompt
+  (if (fboundp 'format-prompt) #'format-prompt ;New in Emacs≥28
+    (lambda (msg default)
+      (if default (format "%s (default %s): " msg default)
+        (concat msg ": ")))))
+
 (defun cperl-info-on-command (command)
   "Show documentation for Perl command COMMAND in other window.
 If perl-info buffer is shown in some frame, uses this frame.
@@ -6332,7 +6338,7 @@ cperl-info-on-command
   (interactive
    (let* ((default (cperl-word-at-point))
 	  (read (read-string
-		 (format-prompt "Find doc for Perl function" default))))
+		 (cperl--format-prompt "Find doc for Perl function" default))))
      (list (if (equal read "")
 	       default
 	     read))))
@@ -8291,7 +8297,7 @@ cperl-perldoc
   (interactive
    (list (let* ((default-entry (cperl-word-at-point))
                 (input (read-string
-                        (format-prompt "perldoc entry" default-entry))))
+                        (cperl--format-prompt "perldoc entry" default-entry))))
            (if (string= input "")
                (if (string= default-entry "")
                    (error "No perldoc args given")




^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: cperl-mode: Eliminating references to obsolete packages
  2020-09-23 19:50     ` Stefan Monnier
@ 2020-09-24 15:34       ` Harald Jörg
  2020-09-24 17:09         ` Stefan Monnier
  0 siblings, 1 reply; 8+ messages in thread
From: Harald Jörg @ 2020-09-24 15:34 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

Stefan Monnier writes:

> [...]
>> I am unsure about the procedure: Should the backwards-compatible
>> version live in a separate branch on Emacs?
>
> I think we don't need a separate branch.  E.g. for format-prompt we can
> use a patch like the one below.  If you can't think of a simple way to
> preserve backward compatibility in a specific situation, feel free to
> ask for help,

Thanks for the patch!  It does the trick, so I apologize that I've
still doubts about the _process_.  I guess we should start making
cperl-mode.el a "dual life" project soonish - I'll better start a new
thread for that.

In detail:

The point of *this* post was to actually _eliminate_ some fboundp
conditionals in cperl-mode.el, so it feels a bit weird to introduce a
new one in the same step.

I am not particularly concerned about the use of format-prompt,
because the calling functions have other issues (cperl-info-on-command
is broken anyway, and cperl-perldoc is based on man, which means it
fails on Windows and has suboptimal formatting on all platforms).
That's outside the scope of this thread, though.

But replacement of deprecated / old stuff does occasionally happens
across the Emacs repository.  This may hit cperl-mode.el at any time,
like it did with time-convert and format-prompt.  So, every now and
then, the Emacs repository will contain a cperl-mode.el which won't
work with Emacs 26, until someone notices it and someone prepares
another backporting-patch.  Or until cperl-mode.el gets a place or
procedure where these changes don't hit.
--
Cheers,
haj




^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: cperl-mode: Eliminating references to obsolete packages
  2020-09-24 15:34       ` Harald Jörg
@ 2020-09-24 17:09         ` Stefan Monnier
  2020-09-24 18:43           ` Harald Jörg
  0 siblings, 1 reply; 8+ messages in thread
From: Stefan Monnier @ 2020-09-24 17:09 UTC (permalink / raw)
  To: Harald Jörg; +Cc: emacs-devel

> The point of *this* post was to actually _eliminate_ some fboundp
> conditionals in cperl-mode.el, so it feels a bit weird to introduce a
> new one in the same step.

Either way is fine by me.

> But replacement of deprecated / old stuff does occasionally happens
> across the Emacs repository.

Yes.

> This may hit cperl-mode.el at any time, like it did with time-convert
> and format-prompt.  So, every now and then, the Emacs repository will
> contain a cperl-mode.el which won't work with Emacs 26, until someone
> notices it and someone prepares another backporting-patch.

That's right.  We have that problem for all 17 current GNU ELPA packages
distributed directly from the `emacs.git` repository.

For example that "format-prompt" problem currently also affects the
python.el GNU ELPA package.  Luckily, AFAICT we haven't yet released
a new python.el package (because the version number has not been
bumped), but the problem is real.

In practice it hasn't been too problematic, in part because it's very
easy to make/distribute/download/install a new release that fixes
the problem.

> Or until cperl-mode.el gets a place or procedure where these changes
> don't hit.

Some automated tests that try to build&compile (and treat warnings as
errors) those packages on the corresponding oldest Emacs version
supported would come in handy.


        Stefan




^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: cperl-mode: Eliminating references to obsolete packages
  2020-09-24 17:09         ` Stefan Monnier
@ 2020-09-24 18:43           ` Harald Jörg
  2020-09-24 22:20             ` Stefan Monnier
  0 siblings, 1 reply; 8+ messages in thread
From: Harald Jörg @ 2020-09-24 18:43 UTC (permalink / raw)
  To: emacs-devel

Stefan Monnier writes:

> For example that "format-prompt" problem currently also affects the
> python.el GNU ELPA package.  Luckily, AFAICT we haven't yet released
> a new python.el package (because the version number has not been
> bumped), but the problem is real.

I noticed that some of them (but not python.el) contain a comment like
this:

  ;; This is an Elpa :core package. Don't use functionality that is not
  ;; compatible with Emacs 24.1.

...but I don't know whether that actually helps.  For me it would be
rather cumbersome to find out which functionality isn't compatible
with Emacs 24.1.  Some recent doc strings (e.g. time-convert) show it,
some (e.g. format-prompt) don't.

> [...]
> Some automated tests that try to build&compile (and treat warnings as
> errors) those packages on the corresponding oldest Emacs version
> supported would come in handy.

Wouldn't this rule out the type of fallback patch you suggested?  When
I byte-compile-file cperl-mode.el in Emacs 26.1 _with_ your fallback
patch applied, I still get the warning:

    In end of data:
    cperl-mode.el:8736:1:Warning: the function ‘format-prompt’ is
    not known to be defined.

Sorta obvious, though: The compilation step doesn't _run_ the fboundp
safeguard.

Anyway: Are there frameworks for such tests in the Emacs repository?
On GitHub, I can apply stuff from
https://github.com/marketplace/actions/emacs-lisp-check, but as of now
this also gives false positives:

    github-actions / test-on-self (26.1, true)
    cperl-mode.el#L5880
    You should depend on (emacs "27.1") if you need ‘cperl-force-face’.

This is fairly idiotic since "my" cperl-mode.el itself _defines_
cperl-force-face.  Somehow the check seems to "know" what's
available in vanilla Emacs 26.1 and 27.1.  I discovered that GitHub
action in Sacha Chua's blog
https://sachachua.com/blog/2020/06/2020-06-22-emacs-news/ and just
gave it a try without actually understanding what I was doing :)
-- 
Cheers,
haj



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: cperl-mode: Eliminating references to obsolete packages
  2020-09-24 18:43           ` Harald Jörg
@ 2020-09-24 22:20             ` Stefan Monnier
  0 siblings, 0 replies; 8+ messages in thread
From: Stefan Monnier @ 2020-09-24 22:20 UTC (permalink / raw)
  To: Harald Jörg; +Cc: emacs-devel

>> For example that "format-prompt" problem currently also affects the
>> python.el GNU ELPA package.  Luckily, AFAICT we haven't yet released
>> a new python.el package (because the version number has not been
>> bumped), but the problem is real.
>
> I noticed that some of them (but not python.el) contain a comment like
> this:
>
>   ;; This is an Elpa :core package. Don't use functionality that is not
>   ;; compatible with Emacs 24.1.
>
> ...but I don't know whether that actually helps.

It helps in the sense that at least the information is available without
having to ask someone, but for things like format-prompt applied via
searches like `grep` it's all too easy to overlook.

> For me it would be rather cumbersome to find out which functionality
> isn't compatible with Emacs 24.1.

That's yet another difficulty, indeed.

> Some recent doc strings (e.g. time-convert) show it,

It's actually not the docstring, it's a hack in the `C-h o` code which
adds this info by searching for the name in the etc/NEWS* files.

> some (e.g. format-prompt) don't.

Indeed.  Should be fixed now in `master`.

>> [...]
>> Some automated tests that try to build&compile (and treat warnings as
>> errors) those packages on the corresponding oldest Emacs version
>> supported would come in handy.
>
> Wouldn't this rule out the type of fallback patch you suggested?  When
> I byte-compile-file cperl-mode.el in Emacs 26.1 _with_ your fallback
> patch applied, I still get the warning:
>
>     In end of data:
>     cperl-mode.el:8736:1:Warning: the function ‘format-prompt’ is
>     not known to be defined.
>
> Sorta obvious, though: The compilation step doesn't _run_ the fboundp
> safeguard.

Just use `'format-prompt` instead of `#'format-prompt`.
But yes, turning warnings into errors can be tricky on older Emacsen.
There are other solutions, tho (e.g. keep an "expected output" against
which to compare so we get warned when a *new* warning appears).

IOW, the problem for now is to setup those automated tests: finding
a machine on which to run them, figuring out which version of Emacs to
use for each, making sure the corresponding Emacs version is available,
etc...

> Anyway: Are there frameworks for such tests in the Emacs repository?

I'm not very knowledgeable about that, I'm afraid.


        Stefan




^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2020-09-24 22:20 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-23 18:37 cperl-mode: Eliminating references to obsolete packages Harald Jörg
2020-09-23 19:16 ` Stefan Monnier
2020-09-23 19:38   ` Harald Jörg
2020-09-23 19:50     ` Stefan Monnier
2020-09-24 15:34       ` Harald Jörg
2020-09-24 17:09         ` Stefan Monnier
2020-09-24 18:43           ` Harald Jörg
2020-09-24 22:20             ` Stefan Monnier

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).