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

Command line interface #8

Closed
wants to merge 2 commits into from
Closed

Conversation

igorshubovych
Copy link
Contributor

  • commander package to run commands.
  • .editorconfig

I can also add the info about it in README.md

It fixes #7.

end_of_line = lf
charset = utf-8
trim_trailing_whitespace = true
insert_final_newline = true

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ironic that this file is missing a trailing newline...right after insert_final_newline = true 😉

@DavidAnson
Copy link
Owner

Wow, this looks great!

I've been thinking about how to add command-line support ever since #7 was opened. This is definitely something I'd like to do, but I've been trying to balance it against my goal to have a minimal set of dependencies (currently just markdown-it). I figured that CLI support would require at least a couple of new dependencies and wanted to avoid that for scenarios that don't take advantage of it (i.e., all current uses). What I eventually decided was to create a separate project/package for the CLI that depends on this project.

I had planned to create that project next, but before I got a chance to do so you submitted this pull request. :) I think what you've done is fantastic, and am not sure about the new dependencies, so I have an idea: What do you think about creating a separate project/package for the CLI based on what you've already written? I would link to your CLI from this README and anyone who's interested would be able to install it!

If you like the idea, I already have a couple of (minor) feature requests ready. :) Or if you do not want to take on ownership of a new project, I could create one and you could resubmit your pull request there.

Please let me know what think - I'm excited to have a command-line interface to markdownlint and think we can make either of these approaches successful!

@jawshooah
Copy link

wanted to avoid that for scenarios that don't take advantage of it (i.e., all current uses)

Well, no current uses take advantage of it by definition, since it doesn't exist yet 😉

A CLI would be very useful for non-node projects that don't have a grunt/gulp build workflow. For example, overcommit could benefit from a pre-commit hook that runs markdownlint on modified Markdown files (though I've already opened a PR to do the same with mdl).

@jawshooah
Copy link

SublimeLinter could also benefit from a markdownlint plugin, which could only exist if a CLI were provided.

@DavidAnson
Copy link
Owner

To be clear, I'm totally in support of adding a CLI for markdownlint! My comment above is just that some scenarios don't need the CLI, so I'd rather avoid forcing the corresponding dependencies onto those scenarios. (Note that in this case the number of required dependencies went from 1 to 5.) Having a separate project for the CLI is a nice way to have your cake and eat it, too. :)

@igorshubovych
Copy link
Contributor Author

I probably can rewrite it with argparse instead of commander. argparse is already used by markdown-it. But it also uses lodash.

@igorshubovych
Copy link
Contributor Author

However there are high chances that commander, rc and so lodash and deep-extend as their dependencies are already used in any node project, because they are often used in some linters (similar to markdownlint) and similar projects.

@DavidAnson
Copy link
Owner

I tried to do a bit of research on this topic today; I found just one discussion of the topic (please link to others if you find them): sindresorhus/ama#17

My read of the comments in that issue (as well as the linked issues) is that there good reasons to have a separate project for the CLI. So that remains my preference for markdownlint.

That said, if you have thoughts on why the CLI should be bundled together with markdownlint, I'm keeping an open mind. :)

Thanks for the interesting discussion!

@igorshubovych
Copy link
Contributor Author

That's was definitely an interesting discussion. Thanks for sharing.

So I published version 0.0.1 of CLI for it.
https://www.npmjs.com/package/markdownlint-cli
https://github.com/igorshubovych/markdownlint-cli

I promise to continue working on it.

@DavidAnson do you want me to add you as a collaborator?

@DavidAnson
Copy link
Owner

Awesome! :)

I will link to your project from the README later today and give it a try as well.

Regarding the ideas I had, would you like me to create issues for them in your project, mention them here, or something else?

do you want me to add you as a collaborator?

I would be honored if you did, but it's completely up to you.

@igorshubovych
Copy link
Contributor Author

Regarding the ideas I had, would you like me to create issues for them in your project, mention them here, or something else?

Whatever you prefer. markdownlint-cli is just a thin wrapper over markdownlint, so if the issue is related to it, then yes, definitely create an issue.

BTW, I added you as a collaborator on Github repo and npm package. Just in case I disappear, and something needs to be addressed.

@igorshubovych
Copy link
Contributor Author

So I am closing this one.

@DavidAnson
Copy link
Owner

Excellent, thank you!

I created a few issues from the ideas I had - you're welcome to agree or disagree with each of them. :)

Also, thanks for the pull request to update the README, I merged it and moved markdownlint-cli to the top of that section.

@DavidAnson
Copy link
Owner

By the way, I will tweet markdownlint 0.1.0 tomorrow and then markdownlint-cli the next day.

@igorshubovych igorshubovych deleted the cli branch March 13, 2016 18:47
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.

A command line interface
3 participants