zenitani / elisp

15 stars 10 forks source link

proposal to extend `smart-compile` to flexibly identify build systems in monorepos #9

Closed cosmicexplorer closed 3 years ago

cosmicexplorer commented 3 years ago

Hello! I have been using your package for years! It's great!

Upon realizing it was on MELPA (I had just copied the file into my own emacs init before), I tried to see what I had added to it. I think I have found a really cool way to extend the package, but not by too much.

Problem

This tries to solve two problems:

  1. I want to run smart-compile with make, but if my Makefile isn't right next to the file (e.g. ../Makefile, in a parent directory), it fails.
  2. I want to run smart-compile with more than just make.

These two would happen, for example, in a "monorepo", where I have used it for years at Twitter to smartly compile with pants

Solution

  1. Now, if there is a Makefile in any parent directory, smart-compile will ask, for example:

    ../Makefile is found. Try 'make ' in its directory?

  2. There's a new defcustom smart-compile-build-system-alist, which tries to match the behavior of smart-compile-alist, but instead to run on a "build system file":
    • Alist of (REGEX . STRING) or (REGEX . SEXP).
    • The "build system file" is any non-directory file matching REGEX in any parent directory,

      Result

    • smart-compile will cd into the directory with the generalized "build system file".
    • If the matching entry in smart-compile-build-system-alist is a (REGEX . STRING), it performs the same format string substitution as specified in smart-compile-replace-alist, where the file is set to the "build file".
    • In addition to make, it also has added support for cargo and ./pants.

Note: we may want to add another defcustom to limit how many parents it will check? I just ended up using 4 for all of my choices in the code I just deleted: https://github.com/cosmicexplorer/.emacs.d/commit/36079382ede69719058ee048ef57ee8623c3fc38#diff-d7cee76abae5125a7d2d1a746d9e194c93f4ddf48685eac5774ff84a8ff181a5L277-L285.

Thank you for your work!

zenitani commented 3 years ago

Thank you for your submission. Can you give me some time to understand the background? It appears that "Rakefile" and "Gemfile" could be moved from smart-compile-alist to smart-compile-build-system-alist. Thanks! Seiji

cosmicexplorer commented 3 years ago

Thank you for your submission. Can you give me some time to understand the background?

Sure! No worries.

It appears that "Rakefile" and "Gemfile" could be moved from smart-compile-alist to smart-compile-build-system-alist. Thanks! Seiji

Absolutely! And in fact that exposes maybe a new concern: do we want to be able to allow multiple fallbacks?

Because we happen to have build systems build off of other build systems:

So I'll first: (1) Move Rakefile and Gemfile to smart-compile-build-system-alist!

In my own personal defcustom I haven't done that super successfully. I did add support for coffescript, but then started using Cakefiles (like Rakefiles) recently. Emacs coffee-mode has a way to configure running coffeescript to get compiled javascipt, I think, and I think the REPL is nice.

However, even with those packages, smart-compile was able to do these one-shot runs of packages really nicely.

I will create another issue or PR to discuss how to handle fallbacks (like, choosing between build sytems at greater depth in the filesytem hierarchy?

cosmicexplorer commented 3 years ago

And like I think it could even make sense to perhaps consider merging smart-compile-alist and smart-compile-build-system-alist maybe? But i didn't want to break existing users by changing the alist format I think? So wanted to consider this perhaps experimental change to try to make it smaller.

cosmicexplorer commented 3 years ago

Although, looking at this again because I think Gemfile and Rakefile might be in the same directory? Would we want to go through all matching alist elements?

("Gemfile\'" . "bundle install") ("Rakefile\'" . "rake")

E.g.:

./Gemfile was located. Want to do bundle install? (y/n) n ./Rakefile was detected. Want to do rake (y/n) n ruby myfile.rb

And C-u 4 can reset it as usual, so that could help to automate like all the different setup steps needed for those systems.

zenitani commented 3 years ago

To avoid a multiple fallback in a very deep directory tree, I would like to introduce a new variable smart-compile-directory-depth or something like that. Its default value would be 0 or 1. A power user can increase it to 10 or more, but he/she can deal with the deep-directory situation at his/her own risk.

zenitani commented 3 years ago

./Gemfile was located. Want to do bundle install? (y/n) n ./Rakefile was detected. Want to do rake (y/n) n ruby myfile.rb

I prefer to go through all matching alist elements. The user will see such many options only at the first time. From the second time, smart-comile runs ruby myfile.rb. Also, the user can increase/reduce the options by editing smart-compile-build-system-alist.