Some refactoring

[swsnr]

Hello,

I'm the Flycheck maintainer, and was recently made aware of this package. I looks really awesome, and is a great piece of work, but I see some issues in the implementation, which I'd like to fix.

There are some smaller ones, such as unprefixed functions (e.g. parse-json), which are discouraged in Emacs Lisp due to the lack of any namespacing, but most notably I'd like to rewrite how you pick the tools to check a buffer with.

Flycheck already has support for multiple syntax checkers per buffer, so ideally this library would provide a separate syntax checker for each tool, and rely on Flycheck's infrastructure to enable/disable specific tools.

What do you think?

[pnf]

Hi -

The prefixing is fixed in an unpushed commit. (You're not the first to have pointed this out!)

If you want to make the tool-picking improvements, I certainly won't object; alternatively, if you prefer to give me some hints, I can make a stab at it myself.

Note that the repo is in the process of being moved to the clojure-emacs project, and some of the team permissioning is still being hashed out, so in any case it may be best to hold off a few days.

[swsnr]

@pnf I can do that myself. It's easier this way, I think, since I know the ins and outs of Flycheck :)

I'll wait until you have sorted out the move to clojure-emacs. Suits me fine, the days before Christmas are busy enough :)

[pnf]

The move is complete. Go wild.

[swsnr]

@pnf Oh, that was much faster than I expected :) I'll try to get to it tomorrow, and open a PR with some initial draft.

I'll need your help on this, though. I don't use clojure, so I'd like to ask you to thoroughly review and test my changes. I'll install clojure, and do some local testing, but since I don't really know the language, my testing will be very superficial at best.

[pnf]

Well, it's actually not hard to move a repo, but they make it seem scary, so the biggest obstacle was convincing myself that I could repair things if it all vanished into the ether. Would it be easier to mock the Clojure interaction? I could commit a few JSON-ized value strings for you.

[swsnr]

@pnf I don't know. It shouldn't be hard for me to install Clojure—I succeeded at this at least once in the past :)

The problem is rather that I'll have a hard time to come up with code that actually triggers some errors or warnings from these linters, because I don't really know a lot of clojure beyond the basic syntax. If you could commit some simple Clojure examples that have errors or warnings with these linters, that'd help me much. I could then also use these examples to write some very basic unit tests for this library, and possible even add Travis CI to it.

[pnf]

There's a sample-project subdirectory that might be useful to your. Mostly, it's there to verify that I'm not making excessive assumptions about a user's Clojure project, but its core.clj demonstrates a (very) few lint objections. I'll expand it, among other things to include kibit warnings.

[purcell]

There's a MELPA PR open for adding these checkers there, and I've suggested in the comments there that we should have separate checkers for the different tools, possibly even packaged individually.

I'll probably hold off on merging that PR right away, because I think we should first figure out how best to package things.

[pnf]

I hadn't realized this wasn't in melpa yet. I'll un-merge the corresponding README PR...

---

Sent from my VAX-11/780

On Dec 20, 2014, at 14:04, Steve Purcell notifications@github.com wrote:

There's a MELPA PR open for adding these checkers there, and I've suggested in the comments there that we should have separate checkers for the different tools, possibly even packaged individually.

I'll probably hold off on merging that PR right away, because I think we should first figure out how best to package things.

— Reply to this email directly or view it on GitHub.

[purcell]

I'll un-merge the corresponding README PR...

Well, no great rush -- it could still go straight in, but I wanted to find out more about the plans for this code first.

[purcell]

My main reservation was that this really needs a semantic name like flycheck-clojure, but given that it also needs its monolithic checker splitting up into three individual checkers, maybe what we should end up with is 3 individual checker packages, e.g. flycheck-clojure-eastwood.

[pnf]

Let's keep it out of melpa for now; the discipline of installing the .el yourself is a good reminder that things are in flux. I'm not sure about 3 separate packages, because (1) there will certainly be more checkers, (2) the checker-specific elisp code is tiny, (3) if we also broke out the corresponding clojure translators into their own packages, we'd be be close to a 1:1 ratio of lines to packages. Anyway, I updated the README.

[purcell]

Then my personal suggestion would be to rename this to flycheck-clojure, and keep the checkers together in a single package. And we can work separately on splitting the current checker into separate tool-specific flycheck checkers.

If we do this, there's no reason it can't go straight into MELPA, which will help you get feedback and contributions.

[pnf]

It's not clear to me yet how to make everybody happy here. Moving the package from my account to clojure-emacs was supposed to be preparation for merging it fully into cider. @bbatsov's philosophy, which I actually share, is that installing cider should provide the user with an expansive development environment that doesn't require much in the way of further installation and configuration. Splitting the flycheckers into separate packages will be pointless if cider, on which they all depend for communicating with Clojure, explicitly depends on all of them.

[magnars]

Three flycheckers in one flycheck-clojure seems to do it.

[purcell]

@bbatsov's philosophy, which I actually share, is that installing cider should provide the user with an expansive development environment that doesn't require much in the way of further installation and configuration.

I'm all for easy installation and setup, but that doesn't imply that everything which depends on Clojure should just get merged into cider. Packages are easy to browse and install, and the dependency mechanism works well, so I would strongly advise against jamming these flycheck checkers directly into cider for the sake of it.

It would be preferable for the cider docs to simply recommend that flycheck users install flycheck-clojure too. There will be people out there who don't like flycheck, and there's absolutely no need to force its installation on them, just as it would be inappropriate for cider to start assuming everyone wants to use company or yasnippet.

[pnf]

Closing this issue as the refactoring PR has been merged; I'll open a new one for the Melpa PR. If anyone wants to continue the debate on further integration with or engulfment by Cider, that can also be a new issue. (On consideration, I'm happy either way.)