gacelita / ventas

Clojure ecommerce platform
Eclipse Public License 1.0
124 stars 14 forks source link

Emacs compat #3

Closed vemv closed 6 years ago

vemv commented 6 years ago

Working fine with these ✌️

I tried to keep the PR minimal in scope. During the debugging process I came up with other little fixes, but they don't seem necessary anymore so I discarded them.

Changes explained per-commit

gacelita commented 6 years ago

Commit by commit: Add nrepl middleware, for Emacs usage ok


Make embed Figwheel server optional, enable by default Please use (config/get :ventas-skip-embed-figwheel-server), add a default to ventas.config/defaults (optionally explain it there), and either edit your resources/config.edn or use an env var called VENTAS_SKIP_EMBED_FIGWHEEL_SERVER. More info: https://github.com/tolitius/cprop


Avoid various repl.clj pitfalls for Emacs users Please remove ventas.core dependency, as this triggers compilation of the source code on repl startup, possibly making it fail.

REPL should always start up, even if you have syntax errors under src. This way you can fix them and keep trying to reload the code, without ever restarting the REPL. A project such as this takes a long time to start, better to do it once. Remember you are supposed to execute (init) right after starting the REPL, to load the source code. Also, ventas.core is meant for production builds exclusively.

Did you run into any problems with this setup?


Avoid ':as gen'' (with single-quote) naming, as it can be problematic: ok


Don't entirely swallow an exception: ok


Avoid sporadic errors caused by '(close! nil)': ok

vemv commented 6 years ago

cprop OK 👍

I checked again and for getting things working (and warn-free), I have to choose one of these fixes:

(I also could do both, but it would be redundant)

I can do a conditional (require 'ventas.core) guarded by cprop. Sounds good?

gacelita commented 6 years ago

Conditional sounds good to me :) but, alternatively, use profiles.clj to edit the :repl-options in project.clj, and choose another ns for starting your repls (instead of user). Or you could toggle that option using an env var instead of profiles.clj, that's your choice (btw you are limited to env vars in project.clj, can't use cprop there)

vemv commented 6 years ago

Rectification:

I need both the require and removing "dev". The latter prevents this:

image

gacelita commented 6 years ago

Fine with me, but bear in mind that dev will not be reloadable if you do that (at least for emacs users). No idea what could be causing that behavior :(

vemv commented 6 years ago

Branch is ready again!

With CIDER one can force-require any given ns (and deps, recursively), which compensates for the dropped dev

gacelita commented 6 years ago

Left two comments - do those and it's merged. Very glad to have it working for emacs!

vemv commented 6 years ago

🍻 ready again (got addicted to the edit option of interactive-rebase btw)