Multiple issues around Emacs package conventions

[lassik]

This package was an early submission to MELPA and doesn't follow modern packaging conventions. Do you accept PRs to fix that?

The main issue is namespace hygiene. Most urgently, dime.el defines its own versions of common macros without using a dime- namespace prefix for them. Of those, at least when-let is incompatible with the standard definition of that macro and breaks other Emacs packages.

DIME also defines some things using a sldb- prefix; that prefix belongs to the SLIME debugger and DIME should use something else.

Would you be willing to raise the minimum required Emacs version to 25.1 which is four years old? That would get us common macros like when-let via (require 'subr-x).

Other issues:

• Lexical binding should be used throughout.
• Package metadata should be added as headers into the .el files.
• Files and functions/macros/variables should be renamed such that the definition prefixes correspond to the filenames in which they are defined.

I have several packages in MELPA and am happy to assist with all of this stuff if welcome. To not break other packages, at least the namespace hygiene should be fixed.

[housel]

Yes, we would be happy for someone to help modernize this code, and yes, I think requiring emacs 25.1 would be fair.

DIME started as a fork of SLIME, so it might be worthwhile looking at what modernizations have been done to that codebase.

[lassik]

Great, thank you! I sent the first batch of commits.

DIME seems to have two kinds of trees:

• One for compiler notes (dime-compiler-notes-tree.el)
• One that seems like a Smalltalk-style class browser, copied from SLIME (dime-dylan.el -- slime-xref-browser.el --- xref browsing with tree-widget)

How to split these into files? dime-compiler-notes-tree.el is already its own file, but dime-compiler-notes-tree i a very long prefix to have in lots of symbols (the convention is that each file foo-bar.el defines only symbols starting with foo-bar). There are some symbols prefixed dime-tree currently in dime-compiler-notes-tree.el, but this is not consistent. dime-tree.el would make for a good file name and prefix, but if the xref tree is also a tree, we should disambiguate the two kinds of trees somehow.

Maybe split the two trees into dime-note-tree.el and dime-browser.el, with the symbol prefixes dime-note and dime-browser respectively?

dime-repl.el is in pretty good shape. Quite well encapsulated into its own namespace, and dime-repl is a nice symbol prefix.

Since we assume Emacs 25 or newer, the file dime-compat.el can be removed altogether.

How is dylan-optimization-coloring.el integrated into the rest of the package? Is it an optional add-on to dime and/or dylan-mode?

[lassik]

SLDB issue solved. Could someone advise me on what to do about the tree features and dylan-optimization-coloring.el?

[housel]

The dime-note/dime-browser naming seems sensible.

The dylan-optimization-coloring.el code isn't directly integrated; it can be used on its own to read files generated using the compiler's -dispatch-coloring elisp option.

[lassik]

The dime-note/dime-browser naming seems sensible.

I'll send another PR to make that change.

dime-note-tree, right?

The dylan-optimization-coloring.el code isn't directly integrated; it can be used on its own to read files generated using the compiler's -dispatch-coloring elisp option.

If the files are Dylan source code with additional optimization coloring on top, those buffers should use dylan-mode as the major mode and we should have dylan-optimization-coloring-mode as a minor mode. If the files do not use Dylan syntax, then dylan-optimization-coloring-mode should be a major mode.

[housel]

With optimization-coloring-mode, two files are involved. The main file is the Dylan source file, which starts out using dylan-mode. If you compile using the -dispatch-coloring elisp option, for each source file you get an output file (using elisp syntax) that contains annotations for dispatch coloring (described, in the IDE case, in https://opendylan.org/documentation/getting-started-ide/coloring.html#about-dispatch-optimizations). The dylan-optimization-coloring.el code can be run on an existing dylan-mode buffer to replace the major mode's coloring with coloring based on the output from the compiler.

So yes, there should be a dylan-optimization-coloring-mode, it's just that nobody has ever spent the time to restructure the code as a minor mode.

[pedro-w]

@lassik please could you advise on the use of defsetf in dime.el (line 1726). I know it's been an obsolete function for a while but I am now finding on my emacs (27.1) it's being reported as an error

Lisp error: (void-function defsetf)

Thanks!

[lassik]

The documentation says:

See gv-define-expander, and gv-define-setter for better and simpler ways to define setf-methods.

[pedro-w]

I'm confident I can make that change. I suppose the question was more - is that what we should be doing? It's great to have an emacs lisp expert to ask!

[lassik]

@pedro-w Try gv-define-setter first since it's simpler, and if it's not powerful enough, gv-define-expander.

I shouldn't be considered an expert, but as a longtime fan of everything that has to do with Lisp I'm happy to help Dylan :)

[lassik]

The ^L page break markers in the elisp code are somewhat archaic. Are any of you still using them?

[cgay]

I'm a fan of getting rid of them. I think @housel said something positive about them a few years ago when I asked. :-)

[housel]

I prefer to keep the FF characters in the Dylan code (for use with narrow-to-page), but for elisp I don't mind either way.

[lassik]

@pedro-w I think I got the setf expansion converted - at least the compiler and package-lint don't warn about it:

(gv-define-setter ,varname (store &optional process)
  `(dime-with-connection-buffer (,process)
     (setq (\, (quote (\, real-var)))
           (\, store))))

[pedro-w]

That's the way I did it. I do get a number of other warnings (a lot of them are to do with putting cl- prefixes on things) - do you want me to file issues on them? I've held off because there's so much change going on else where!

[pedro-w]

(indeed you may have already fixed them in other commits)

[lassik]

@pedro-w Yes, that version of the setter code is now committed.

As far as I can tell, everything this issue is now taken care of. Thanks to everyone who participated!