-
Notifications
You must be signed in to change notification settings - Fork 450
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
Switch to remark-cli to support .remarkrc and unlock remark version #2008
Conversation
@wooorm there's one thing that's working not as I've been expecting and I'm not sure if this is a peculiarity of When I define a rule (e.g. What would you suggest? |
"use strict" | ||
Beautifier = require('./beautifier') | ||
|
||
module.exports = class Remark extends Beautifier | ||
name: "Remark" | ||
link: "https://github.com/wooorm/remark" | ||
link: "https://github.com/remarkjs/remark" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The project has been moved to an org in ≈ Dec 2017
serializedSettings = JSON.stringify(settings).slice(1, -1) | ||
remarkArgs.push('--no-config', '--setting', serializedSettings) | ||
|
||
return new Promise((resolve, reject) => |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A new promise object is not really necessary here at this point, but it is helpful to integrate the new implementation of the beautifier with #1990
Yes, remark-lint just warns, it doesn’t change anything. remark-stringify (used in remark-cli, by the remark library itself, and here as well) does support some settings to beautify a certain way. There’s also no code yet to map lint rule settings to remark-stringify settings, but that would be useful for other tools as well! Alternatively, prettier can now format markdown as well, and it’s based on remark. It has a very nice line wrapping algorithm, that remark itself doesn’t have yet! Does that help? Let me know if I can be of more help! |
Thanks for the explanation @wooorm. Hmmm I did not know that prettier can work with markdown via remark, interesting. To be honest, my overall goal is to come up with an environment that’s similar to what prettier-eslint provides in atom. There I’ve got some rules defined in |
ping @wooorm :–) |
Sorry for the slow response! You need some custom function, I guess? I’m not sure, it would need some work figuring out what remark-stringify settings correspond to remark-lint rules. How can I help? |
@wooorm sorry I was tagging you in the wrong thread, it should have been here. Can remark (based on unified) format markdown and support a |
.remarkrc files are only supported by remark-cli. See |
@wooorm so can we somehow use |
Well, the You could also allow the plugins. That could allow weird things though. Say someone has a pipeline to transform markdown to HTML, if atom-beautify respected that config, all files would end up as HTML! But first: why do you want |
I'd like to enable the same functionality as prettier-eslint provides. That is to allow a team to deviate from the 'industry standard' linting rules but still be able to autoformat the source files. Someone's editing policy may require When I contribute to a third-party project documentation, I'd like to be able to make their linter happy in one key press rather than by fiddling with micro-editing of symbols and spaces (I may have a habit of padding lists with two spaces, but that's not what the community I contribute to expects). Such a situation is when Of course, in an ideal world we would all be just using some unified formatting rules, but unfortunately we're quite far from that. Prettier has been trying to standardise JS files for some time but that still does not work well and ends up with lots holy wars in their issues on github 😄 |
Well, in that case Now, the
Other plugins, like the People are free to set up remark lint rules exactly like the compilation settings (or not, but that’s weird). However, I don’t think that should be a concern to Anyway: the best way to do this is by depending on |
I agree that |
What’s wrong with |
Could it be that people add |
@kachkaev I checked out your branch locally and, apart from |
You could make a I have both formatting stuff like remark-toc ( |
@wooorm yeah the @szeck87 |
A side-thought: is any preference given to a locally installed |
@kachkaev It won't read any locally installed version at all. I believe it is possible to do so, but it requires a lot of new code since it checks the version of the executable before it even hits the beautify function. |
Right. Do other beautifiers give preference to locally installed executables? Or is this something against |
The closest thing I can find is cljfmt and it's completely out of date (using |
Maybe we could use a global-only instance of remark for now then. However, may be a good practice to search for an executable in project deps first and give that instance a preference. Have seen in a couple of places; prettier-atom is on top of my head. |
Where in prettier-atom specifically? |
That just looks like it won't format if you don't have prettier as a dependency in the directory of where you're working. Anyhow, like I mentioned earlier, ideally we don't use executables unless there is no other alternative. If you can find a way to get the cli version to run within node, that would be great. Otherwise I'll defer . to @Glavin001 on what to do. |
Agreed! Related to remarkjs/remark-lint#82! |
I would agree with @szeck87 .
However, in this case using This route is acceptable as long as users still have the option to configure via |
@@ -185,7 +185,6 @@ | |||
"open": "0.0.5", | |||
"prettydiff2": "^2.2.7", | |||
"pug-beautify": "^0.1.1", | |||
"remark": "6.0.1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kachkaev @szeck87 : I think we can support both Remark-CLI and be backwards compatible with remark
as a Node.js dependency.
Since the Remark CLI
is now an Executable we should be able to determine if it is installed or not.
See
atom-beautify/src/beautifiers/executable.coffee
Lines 99 to 107 in 30dece2
@isInstalled = true | |
@version = version | |
) | |
.then((version) => | |
@info("#{@cmd} version: #{version}") | |
version | |
) | |
.catch((error) => | |
@isInstalled = false |
I'm thinking something along the lines of:
isCliInstalled = @exe("remark").isInstalled
if isCliInstalled
# ...
@exe("remark").run(remarkArgs...
# ...
else
# ...
cleanMarkdown = remark().process(text, options).toString()
# ...
I would prefer to add new features while being backwards compatible. Removable of remark
as a builtin dependency is considered a breaking change to me. However, there is value in using Remark CLI
directly 👍, such as the configuration file.
@Glavin001 will you have time to finish these changes by a chance? I'm afraid I'll be quite busy for the next several weeks. Feel free to take over this PR and merge it! And thanks for your review! |
This issue has been automatically marked as stale because it has not had recent activity. If this is still an issue, please add a comment. It will be closed if no further activity occurs. Thank you for your contributions. |
@kachkaev do you still want to move forward with this, or just use Prettier combined with your other PR for formatting code blocks in markdown? |
Hi @szeck87! I have switched to Prettier and am currently betting on its use with plugins. I won't be able to finish this PR but you are free to continue with its implementation. |
OK no problem, I'll close this out. |
What does this implement/fix? Explain your changes.
I've been trying to add
.remarkrc
support to fix #965 and was suggested by @wooorm to have a look atremark-cli
. As a result, I re-implemented remark beautifier to useremark
as an external tool.The benefit of this approach is not only a local
.remarkrc
file can be found and loaded, but also that there is no more version lock-in forremark
(it used to be v6, which is 2 versions behind the latest).I don't pretend that this PR is perfect and will be happy to revise it with someone's help.
Does this close any currently open issues?
Closes #965. Related to #2004.
Checklist
Check all those that are applicable and complete.
master
branchnpm run docs
CHANGELOG.md
under "Next" section