cider icon indicating copy to clipboard operation
cider copied to clipboard

Move source files under a dir?

Open vemv opened this issue 11 months ago • 9 comments

I noticed that when one visits https://github.com/clojure-emacs/cider on a browser, the README is buried by an abundance of source files.

As a counter-example, I found it very refreshing to visit https://github.com/minad/corfu and see the README header right there - no scrolling needed.

Perhaps we could store the .el files under lisp/ like e.g. https://github.com/magit/transient does?


NOTE: do this with caution, since the .sh scripts must keep being exported to Melpa artifacts.

vemv avatar Sep 22 '23 10:09 vemv

Yeah, I agree that'd a good idea. I've been thinking for this for a while, but I kept putting it off as it wasn't particularly important. Gone are the days when CIDER was exactly one Elisp file...

bbatsov avatar Sep 22 '23 13:09 bbatsov

The lisp/ folder idea is fine by me. I've noticed some projects do something slightly different - e.g. https://github.com/slime/slime I'm not sure if there are any best practices for how a bigger packages should be organized in terms of folders, but I'm never concerned to follow @tarsius's example.

bbatsov avatar Sep 22 '23 13:09 bbatsov

Most multi-library packages probably put the libraries at the top-level, but among those that use a subdirectory, lisp/ seems to be the most common. You could grep the Melpa recipes for :files to learn what other directories are being used and how common that is. However, no other subdirectory is used commonly enough to warrant adding support to Melpa's (i.e., package-build.el's) default files spec. If you use lisp/ you won't have to set :files in the recipe, if you use another subdirectory, then you will have to.

In short, yes, use lisp/. :grin:

tarsius avatar Sep 22 '23 14:09 tarsius

@tarsius Thanks for the explanations of the subject!

bbatsov avatar Sep 29 '23 05:09 bbatsov

@vemv Let's just move everything under lisp/ after we cut CIDER 1.8.

bbatsov avatar Sep 29 '23 05:09 bbatsov

This is looking somewhat delicate. Moving the dirs would work for melpa, but we'd have to look into nongnu elpa as well.

One also has to keep Eldev working (I couldn't immediately find a way to add lisp/ to the working source paths)

Finally, I'd be afraid of breaking git-based consumers (possibly: spacemacs, evil, straight.el). We could coordinate if necessary.

Maybe by declaring a cider-pkg.el file (doc) one would let consumers auto-discover our structure.

vemv avatar Nov 05 '23 11:11 vemv

One also has to keep Eldev working (I couldn't immediately find a way to add lisp/ to the working source paths)

I guess @doublep can consult us on this. I assume it should be something that's easy to do.

Finally, I'd be afraid of breaking git-based consumers (possibly: spacemacs, evil, straight.el). We could coordinate if necessary.

For me that's not an issue at all - we officially support only package.el and other consumers should be capable of dealing with upstream changes. Pretty sure this won't be particularly disruptive from them anyways, as they'll literally need to change one folder name.

This is looking somewhat delicate. Moving the dirs would work for melpa, but we'd have to look into nongnu elpa as well.

The necessary changes there should be submitted after the code has been reorganized. (see https://git.savannah.gnu.org/gitweb/?p=emacs/nongnu.git;a=blob;f=elpa-packages;h=0f722f7fd59d530690429be2bfde4a447a70d7c8;hb=HEAD#l416) This will break the snapshot build on NonGNU ELPA, but almost no one uses it and people will still have access to the last stable build, so it's not a big deal.

I think things like cider-pkg.el are not in fashion these days - I don't see a single bigger package using those, so probably we should not go this route.

bbatsov avatar Nov 05 '23 20:11 bbatsov

One also has to keep Eldev working (I couldn't immediately find a way to add lisp/ to the working source paths)

I guess @doublep can consult us on this. I assume it should be something that's easy to do.

It's currently not supported, because was never requested. Submit a FR, likely not difficult to implement.

doublep avatar Nov 05 '23 21:11 doublep

@vemv The necessary changes were made in Eldev 1.8 (see https://github.com/emacs-eldev/eldev/commit/f6fdcf8031e5cdf9e92c1ec77ee06f0114c66b27), so you can proceed with this.

bbatsov avatar Dec 14 '23 10:12 bbatsov