Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ISSUE 3 Add .zprintrc support #4

Merged
merged 13 commits into from Dec 26, 2018
Merged

ISSUE 3 Add .zprintrc support #4

merged 13 commits into from Dec 26, 2018

Conversation

athomasoriginal
Copy link
Contributor

@athomasoriginal athomasoriginal commented Sep 24, 2018

Whats up?

Thank you @roman01la for putting zprint-clj and zprint-atom together. This is great work! As noted in #3, I too felt the need for the customization power of .zprintrc. As a result, I put together an initial implementation to support this.

M'kay, tell me more

This PR includes the following changes for your consideration:

  • update the build scripts to build for node environments
  • support for the .zprintrc file (the first point outlined in What about zprint configuration? #3 )
  • a local development guide for the README
  • upgrade cljs from 1.9.x -> 1.10.439
  • upgrade zprint from 0.4.6 -> 0.4.13

The implementation to support the .zprintrc file follows the implementation set out by zprint-npm.

I have tested this locally as a CLI app and also in Atom and it seems to work well.

Any guidance you have on this matter would be greatly appreciated.

@ash14
Copy link

ash14 commented Nov 27, 2018

Any updates on this? I've currently got this monkeypatched into my setup and it works quite well as is.

@roman01la
Copy link
Collaborator

@tkjone LGTM. The only thing is that how about looking up config file in project directory as well? I imagine this would be useful for OSS projects where authors might have their preferred config in the repo.

@athomasoriginal
Copy link
Contributor Author

athomasoriginal commented Nov 30, 2018

@roman01la Good point! Do you want me to add this as part of this PR or create a new one?

What do you think about letting the user provide a path to find the config in general. So the lookup could be something like:

  1. check if user passed a path to the config
  2. look in local dir for config
  3. look in global dir for config

@roman01la
Copy link
Collaborator

I think adding to this pr is fine. I'd go with local and global lookups for now, bc these are common options.

ISSUE 3 This will team zprint-clj how to find a .zprintrc file in the
current directory.  The idea is to allow users to have zprint-clj find
and respect project level configurations.  This is a naive solution.  A
better solution would be to have zprint-clj be smarter about how it find
the .zprintrc file
Added a section letting readers know that they can specify customization
in .zprintrc file.  Also updated local development guide to be a little
more clear
@athomasoriginal
Copy link
Contributor Author

athomasoriginal commented Dec 13, 2018

@roman01la Okay, finally got around to implementing current directory (project level) .zprintrc lookup. This is a naive approach and there are much better ones, but this should satisfy the base level
requirement. Let me know what you think.

A few questions

  • Do we want to keep the output of out/* tracked in git?
  • I bumped the cljs and zprint versions to latest. Seems to run without issue. Let me know if this was not what you had in mind and I can revert it.

Cheers!

@roman01la roman01la merged commit f7e846e into clj-commons:master Dec 26, 2018
@roman01la
Copy link
Collaborator

LGTM 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants