lsegal / yard

YARD is a Ruby Documentation tool. The Y stands for "Yay!"
http://yardoc.org
MIT License
1.94k stars 397 forks source link

Default class for syntax highlight #1140

Open noraj opened 6 years ago

noraj commented 6 years ago

By default Yard add automatically a ruby class to all rendered code blocks.

This is a good idea for a ruby documentation tool.

But I think this is the behavior to keep only for ruby files. Because in my markdown files like README.md or INSTALL.md I want that by default code blocks with no specified class don't have one in the rendered html. Yard alone seems no to highlight them anyway but when using a plugin like yard-coderay or yard-pygments, as they have the ruby class they are colored as ruby instead of no having highlight.

So in my opinion the auto adding of ruby class should be only for .rb, .ru, .rdoc, .erb, etc.. and all ruby related extensions but not for markdown files or other supported markups.

Actually for all files the behavior is: add ruby class (in rendered html) if none provided (in source code), add only the class provided in the source code into the rendered html if there is one.

Steps to reproduce

Markdown:

```
$ gem install nvd_feed_api
```

Will render in

Markdown:

```plaintext
$ gem install nvd_feed_api
```

Will render in

Actual Output

Markdown:

```
$ gem install nvd_feed_api
```

Will render in

Expected Output

Markdown:

```
$ gem install nvd_feed_api
```

Will render in

<pre class="code">
    <code>$ gem install nvd_feed_api</code>
</pre>

Environment details:

I have read the Contributing Guide.

lsegal commented 6 years ago

I think it's fair for a Ruby documentation tool to assume that code blocks provided in your project are Ruby code unless otherwise specified. Even in a README, non-Ruby source lines should be fairly minimal and apply only to the few "gem install X" lines, which in practice generates the same overall result since shell scripts don't really have much syntax highlighting anyway.

As you pointed out, you can (and should) override non-Ruby source with plaintext or even just sh, since that's what it is:

```sh
$ gem install foo
```

As a totally separate note, you should probably be doing this for all code blocks, even Ruby code. VS Code actually has a nice Markdown linter that defaults to warning about fenced code blocks that are missing a language specifier. That's a good linting rule to use in my opinion, and avoids this type of ambiguity.

lsegal commented 6 years ago

Sorry for the double post but just to add: it is not possible to change this behavior since it would be a breaking change and would probably affect many projects. I'm afraid the above suggestions are the only route here.

noraj commented 6 years ago

I think it's fair for a Ruby documentation tool to assume that code blocks provided in your project are Ruby code unless otherwise specified.

Yes I agree.

Even in a README, non-Ruby source lines should be fairly minimal

I do not agree, there can be a lot of commands for installation, dependencies, build, updates, etc.. or just code blocks for output of commands. A gem is not necessary a library but can also be a CLI tool.

Your own README contains more non-ruby than ruby code blocks. Same for output of a command:

.

There are a lot of case in a 100% ruby project, where you want non-ruby code blocks in the documentation.

As a totally separate note, you should probably be doing this for all code blocks, even Ruby code. VS Code actually has a nice Markdown linter that defaults to warning about fenced code blocks that are missing a language specifier. That's a good linting rule to use in my opinion, and avoids this type of ambiguity.

I do not agree. No class specified in markdown = like a void class so this is because you don't want a class in the rendered html. For no highlight class CodeRay use plain and plaintext, highlight.js use nohighlight, etc.. so if you begin to add class everywhere your markdown code is no inter-compatible, and in most tool you can add only 1 class in your markdown. So no specifying a class to all code blocks in markdown is not a good practice.

Sorry for the double post but just to add: it is not possible to change this behavior since it would be a breaking change and would probably affect many projects. I'm afraid the above suggestions are the only route here.

So ok not for a revision version or a minor version but for a major version (that by definition bring breaking changes), so why not in yard 1.0.0 ?

lsegal commented 6 years ago

So ok not for a revision version or a minor version but for a major version (that by definition bring breaking changes), so why not in yard 1.0.0 ?

There are no plans to break YARD's API unless there is a very good reason for it. In this case, I don't see a strong reason to change this.

noraj commented 6 years ago

There are no plans to break YARD's API unless there is a very good reason for it.

That's not what I meant. I don't mean the is a feature that need to create a major version right now, I mean because this is a breaking change this feature must wait until the next major release.

In this case, I don't see a strong reason to change this.

It is yard is adding a ruby for all kind of non-ruby code blocks. Example: people who want to write a plugin that transform yard into another language documentation tool (like you did with yard-js) will have automatically a ruby class added to all code blocks where there is no class specified. For example with yard-js you may want the default behavior to add javascript instead. But in most cases you just want nothing if you don't add a class yourself, the only exception are all ruby related extension files.

lsegal commented 6 years ago

That's not what I meant. I don't mean the is a feature that need to create a major version right now, I mean because this is a breaking change this feature must wait until the next major release.

I understood this. There are currently no plans for any major version bumps because there are currently no plans to break any existing APIs (since there are no strong reasons to do this). Breaking an API would be a last resort for YARD. Even if YARD did major version bump, it would not come with any major breaking changes such as this one, because we would need to create a deprecation plan first.

You might see smaller things changed in a version bump (the way settings are configured, for example), but changing a feature that would break a significant amount of existing projects (the ones hosted on rubydoc.info for example) is pretty much a non-starter for this project.

noraj commented 6 years ago

Yeah you're right, it's hardcore for a small benefice. That's true that yard is massively used.

I have another idea. Instead of breaking the default behavior of yard. Why not add an option that can change the behavior and be off by default or having a default value. So nothing breaks for anybody.

Like adding an option --default-code-class or something else. It can have ruby as default value but can be change to ` (nothing, the behavior I talked before) or evenanotherLangfor plugins likeyard-js`. Sounds better?

ghost commented 6 years ago

It would be nice if this could be configured.

Most of my code is ruby specific so I do not need the functionality Alexandre described. But I agree with his point too, e. g. shell-code instructions (may be for rbenv, rvm etc... such projects).

In many ways, markdown is really super-simple, so it does not allow for a lot of modifications (here I refere e. g. to INSTALL.md or README.md which often goes with a .gem file too).

With HTML/CSS, changing this would be super-trivial. People can just use their custom CSS stuff too, to change the layout. :)

I guess markdown is both a blessing and a curse because I also understand Loren's point of view. The net benefit is probably really, really small/negligible. For pure ruby code, similar to rdoc-style documentation, this is probably not really needed in like 99.99% of the time or so.

Alexandre's last comment may be simple to add (e. g. --default-code-class=ruby being the norm, whereas people could overrule this) but to be honest, it may really be such a small use case. Perhaps if there are more people who may need it and could comment on the issue here?