Add draft mode, custom css, and more

yfakariya commented 3 years ago

This PR upgrades preview.js-vscode to 0.19.1 and add some features related it.

With 0.19.1, we can
- Use //table block with inline elements
- Use fence notation for inline elements like @<tt>|void main(){}|
- Use @<chap>{}, @<chapref>{}, and @<title>{}
- Escaping of closing braces support on inline elements like @<tt>{void main(){\}}
- Inproc mode and draft mode describing bellow
- But we lose nested inline elements support, because preview.js-vscode now prohibit it to improve compatibility with Ruby implementation and the spec never allowed nesting.
New feature of the extension
- Optional draft mode support, which shows inline and block comment for editorial purposes. The name is borrowed from the option of Ruby implementation
- Implicitly using inproc mode of the review.js-vscode which prevents verbose log causes by process.exit() call of the compiler.
- Custom CSS specification.

yfakariya commented 3 years ago

A screen shot of the vscode settings window is here:

settings_screenshot

atsushieno commented 3 years ago

Thanks for the PR!

I have a fundamental question: should this "draft mode" even an option? When do we want to have it "false" ?

My understanding is that it should rather be always true. and the formatting (or custom styling) make it distinct from non-comment lines.

On stylesheet: I'm all for having it customizible, but users of this extension, now with public options, would wonder what they can specify to the option, and what the stylesheet content would be like (I have no idea either). If it is an "advanced" option then it should probably be explicitly labelled so.

It also applies to "preview/alpha" feature, if it is. Otherwise, once we make the option publicly available, the setting item name should probably remain "stable" i.e. we cannot really change the semantics of the option with the name.

(I'm assuming this as a preview-ish feature - I'm seeing how VSCode itself lets users choose themes, and it seems that a combo box (an option list) is shown to them and they choose one of the items, where an item is mapped to the actual theme resource which is stored somewhere. If we do something similar, we would just let users choose the right theme without letting them to specigy custom styles.)

yfakariya commented 3 years ago

Thank you for quick reviewing! I am sorry to postpone the discussion of spec to this PR.

whether draft mode is optional

Although I don't have strong intention for it, Ruby implementation actually have --draft option, so it is natural to set draft mode is optional.

styling

I assume that users who want to show preview with their own tweaked css, which can be multiple like style-ebook.css and style-web.css. So, it should be public option to change css file, and I think it is somewhat independent from themes (and it is available to reflect the theme to custom CSS).

Is it reasonable to improve description to explain what and why of this option?

option stability

I completely agree your opinion. We should be careful for compatibility. I will prepend alpha (preview is too confusing for options for previewing as you know) to css option if the above discussion results to CSS file specification should not be stable feature.

atsushieno commented 3 years ago

Thanks for sharing the details.

Ruby implementation actually have --draft option, so it is natural to set draft mode is optional.

Right. But there is a significant difference between the original review and vscode-language-review: review is used to build final production (too), whereas vscode-langauge-review is (so far) only for preview (wherever HTML formatting is involved).

styling

I got the points. The problem is however, there is no formal documentation and stable document structure that makes it possible to write custom styles, partly because it is more of Re:VIEW-to-HTML conversion implementation details. If we intend to let users write custom stylesheets, then there should be some documentation on how to achieve that. And I'm quite unsure if stabilization can be achieved. That's why I thought a style selector option would work in more appropriate way, or at least tell normal users that "it's not for you".

(And my gut feeling is, most people would just want "dark mode" styling, or just applying styles that are created by some other people. That's why I thought it'd be more like a style selector combo box. But it is ignorable, it needs not a small set of UI changes. A simple textbox for a file path is usable enough.)

I will prepend alpha (preview is too confusing for options for previewing as you know)

Thanks, and that's a good catch!

yfakariya commented 3 years ago

Ruby implementation actually have --draft option, so it is natural to set draft mode is optional. Right. But there is a significant difference between the original review and vscode-language-review: review is used to build final production (too), whereas vscode-langauge-review is (so far) only for preview (wherever HTML formatting is involved).

I agree your assumption for the use-case, but I am also afraid that users confuse when inline comment and/or block comment are output to preview because it is actually difficult to provide documentation as far as I understand. How do you think about set default of this draft option to true instead of false?

styling

I got your thoughts. So, I will the option to be alpha and add notice that this is advanced option and subject to change in future.

atsushieno commented 3 years ago

OK, I guessed there is some difference in our premises, and did some investigation on how @<comment>{...} works. Let me know if it seemed wrong.

As I stated at https://github.com/atsushieno/vscode-language-review/issues#issuecomment-725669802, I thought it is rather a matter of emphasizing the comment markup visible on the HTML outcome. It turned out that the original review-webmaker doesn't really give any special formatting on @<comment>{...}. So those Re:VIEW @<comment>{...} users don't really expect them to have remarkable formatting. Therefore if we simply enable --draft equivalence by default, those users would just get confused.

If that's the case, then the best (least-confusing) solution would be to have the option as a feature, and disabling it by default just like how it works in this PR now, and we only need the change for the styling property name.

yfakariya commented 3 years ago

Thank you for arrangement, I understood that I had not understood your premise, I had really forgotten about existence of review-webmaker.

So, I will push fix which will contain only modify setting property name and description.

yfakariya commented 3 years ago

I found that exprementalFeatures should be better than alpha on the screen.

settings_screenshot2

atsushieno commented 3 years ago

That looks even nicer!

yfakariya commented 3 years ago

I've eventually pushed fixed code now!

atsushieno commented 3 years ago

merged and published as v0.7.2 to VS marketplace. Thanks!

atsushieno / vscode-language-review