facelessuser / pymdown-extensions

Extensions for Python Markdown
https://facelessuser.github.io/pymdown-extensions/
Other
957 stars 254 forks source link

Critic viewing issues #95

Closed facelessuser closed 7 years ago

facelessuser commented 7 years ago

EscapeAll escapes placeholder chars STX and ETX which are used to in the Markdown conversion process. These should probably not get escaped. This affects Critic, but also can affect other things.

Also, critic in code blocks aren't having tags inserted. Critic tag insertion should be moved after raw HTML insertion.

RoyiAvital commented 6 years ago

Does critic allow working (Like Accept / Reject, Like ) with CriticMarkup (Like CrtitcMarkup ToolKit for SublimText) or just display it?

facelessuser commented 6 years ago

The documentation answers this: https://facelessuser.github.io/pymdown-extensions/extensions/critic/.

Though I would probably rephrase the documentations statement (now that I look at it today) to the following:

Critic allows you to automatically accept edits, reject edits, or render the output accordingly.

RoyiAvital commented 6 years ago

So the user can't do it in one by one basis? Something like if the caret is on CriticMarkup element and the user clicks Ctrl+A then it is accepted and Ctrl+R to Reject (I just made up the keyboard shortcut).

facelessuser commented 6 years ago

Python Markdown doesn't have a mechanism for interactive input from the user while it is parsing.

If this is something you are interested in, it is probably easy enough to create a simple interactive script that finds and replaces CriticMarkup.

Unfortunately, this is out of scope for this extension. This is just a way to quickly strip CriticMarkup if you know you want to accept them or reject them. You could simply evaluate them by editing the file if you do not want to blanket accept or reject them.

RoyiAvital commented 6 years ago

Hi, You're right. This feature is in the level of the editor and not the parser. Namely adding command of editing text into Sublime Text.

RoyiAvital commented 6 years ago

@facelessuser ,

I found a bug in the Critic extension. I enabled it on the settings and tried this text (From you documentation):

Here is some {--*incorrect*--} Markdown.  I am adding this{++ here++}.  Here is some more {--text that I am removing--}text.  
And here is even more {++text that I am ++}adding.  
{~~ ~>  ~~}Paragraph was deleted and replaced with some spaces.  
{~~  ~>~~}Spaces were removed and a paragraph was added.  
And here is a comment on {==some text==}

This is the result (Chrome 68, Windows 10):

image

It seems the color of the text doesn't work. This is the HTML Code of the file (I never changed the CSS):

https://pastebin.com/3AzmTf9q

I'm not expert on CSS but it seems you define the properties by:

/* Multimarkdown Critic Blocks */
--
  | .markdown-body .critic_mark {
  | background: #ff0;
  | }
  |  
  | .markdown-body .critic_delete {
  | color: #c82829;
  | text-decoration: line-through;
  | }
  |  
  | .markdown-body .critic_insert {
  | color: #718c00 ;
  | text-decoration: underline;
  | }
  |  
  | .markdown-body .critic_comment {
  | color: #8e908c;
  | font-style: italic;
  | }

So I'd expect the class of the addition text would be critic_insert yet it is critic while the element name is ins.
For some reason the browser applies the text-decoration.

So something not working. If you could also explain me what should have happened (Though the classes don't match) I'd also learn something new :-).

Thank You.

facelessuser commented 6 years ago

This from the example CSS? If so, it might be wrong, I'll have to verify.

facelessuser commented 6 years ago

Actually, how are you enabling citric, and for which parser?

facelessuser commented 6 years ago
  1. I think I've mentioned this before, but I need you to create new issues in the future when you think you have a bug.

  2. The CriticMarkup spec is here: http://criticmarkup.com/spec.php, and the output of the critic extension (with example CSS) is described here: https://facelessuser.github.io/pymdown-extensions/extensions/critic/#css.

So are you seeing a discrepancy with the critic extension and the CriticMarkup spec?

Is the CSS you references from Multimar down? If so, I do not promise parity with that, only CriticMarkup.

facelessuser commented 6 years ago

Keep in mind, there is some deviation from the spec with regard to the block class. We also have a couple of workaround/hacks to help in some cases to provide better HTML output, but read https://facelessuser.github.io/pymdown-extensions/extensions/critic/#limitations-with-previewing-critic-markup to understand more about limitations.

RoyiAvital commented 6 years ago

@facelessuser ,

  1. In order to enable CriticMarkup all I did is editing the preferences by adding:
    "pymdownx.critic":
            {
                "mode": "view",
            }
  2. This is the default CSS of your Sublime Text Package.
  3. The bug is the color of the text doesn't match the CSS (Though the text-decoration works, I'd be happy to understand why, it is something I don't get in this CSS code).
  4. The parser doesn't give text addition the class of critic_insert.
  5. The parser doesn't give text removal the class of critic_delete.
  6. The parser doesn't give text mark the class of critic_mark.
  7. The parser doesn't give text comment the class of critic_comment.
  8. The parser use the elements ins, del, comm and mark.
  9. For some reason the mark works as expected.

Again. just enable the Critic extension on default configuration of your Sublime Text Package and see the results I wrote at https://github.com/facelessuser/pymdown-extensions/issues/95#issuecomment-408967600.

facelessuser commented 6 years ago

This is the default CSS of your Sublime Text Package.

Then maybe this needs to be discussed in an new issue over there.

The parser doesn't give text addition the class of critic_insert. The parser doesn't give text removal the class of critic_delete. The parser doesn't give text mark the class of critic_mark. The parser doesn't give text comment the class of critic_comment.

Again, referencing the Critic documentation: https://facelessuser.github.io/pymdown-extensions/extensions/critic/#css., we don't use critic_insert etc. This may have been something I did early on, and probably didn't update the CSS in the Markdown Preview plugin (I assume this is the Sublime plugin this is related to).

RoyiAvital commented 6 years ago

I guess there are 2 options:

  1. Fix the parser for the current CSS of Sublime Text Package.
  2. Fix the default CSS of Sublime Text Package to match the parser.

I guess you'll take the logical choice which is 2. I wonder how come the browser does take the text-decoration property in the image I pasted above.

Anyhow, it needs to be fixed.

facelessuser commented 6 years ago

Fix the parser for the current CSS of Sublime Text Package.

There is nothing to fix here. The Critic extension works as described, unless you've found something?

I wonder how come the browser does take the text-decoration property in the image I pasted above.

The default for ins for a number of browsers is to style with underlines, so most likely, the CSS is not being recognized, but the default style for ins is being used.

Fix the default CSS of Sublime Text Package to match the parser.

Yes, this most likely will need fixing if it is no longer adhering to how the current parser is handling it.

Anyhow, it needs to be fixed.

Please create an issue at the repo so I don't forget. Most likely I will copy over the example CSS shown in the Critic extension's documentation (assuming there are no issues with it), so pull requests are welcome too.

RoyiAvital commented 6 years ago

Hi, I can confirm that updating https://github.com/facelessuser/MarkdownPreview CSS with the CSS of https://github.com/facelessuser/pymdown-extensions/blame/260239559dfdb0ebc957faa41a61ce7578d33a78/docs/src/markdown/extensions/critic.md#L116 solves the problem and the output is colored as expected.

Sorry, I really don't know how to handle Pull Request and building and all that stuff.

There is a feature I can't replicate in the CSS.

image

How did you add that icon? I can see its style content: "\E0B7"; yet when I do that for the CSS (https://github.com/facelessuser/pymdown-extensions/blame/260239559dfdb0ebc957faa41a61ce7578d33a78/docs/src/markdown/extensions/critic.md#L116) the result isn't the same.
I wonder if you can make this icon in the default CSS as well.

Thank You.

P. S. OK, I see it is part of having font-family: Material Icons;.

I managed to replicate this using:

<!DOCTYPE html>
<html>
<head>
<link rel="stylesheet" href="https://fonts.googleapis.com/icon?family=Material+Icons">
<style>
p::before {
    padding-right: .125em;
    color: rgba(0,0,0,.26);
    content: "\E0B7";
    vertical-align: -.125em;
    font-family: Material Icons;
    font-style: normal;
    font-variant: normal;
    font-weight: 400;
    line-height: 1;
    text-transform: none;
    white-space: nowrap;
    speak: none;
    word-wrap: normal;
    direction: ltr;
}
</style>
</head>
<body>

<p>My name is Donald Duck</p>

</body>
</html>

Can be done also by injecting this into the CSS:

@font-face {
  font-family: 'Material Icons';
  font-style: normal;
  font-weight: 400;
  src: url(https://fonts.gstatic.com/s/materialicons/v38/flUhRq6tzZclQEJ-Vdg-IuiaDsNc.woff2) format('woff2');
}

.material-icons {
  font-family: 'Material Icons';
  font-weight: normal;
  font-style: normal;
  font-size: 24px;
  line-height: 1;
  letter-spacing: normal;
  text-transform: none;
  display: inline-block;
  white-space: nowrap;
  word-wrap: normal;
  direction: ltr;
  -webkit-font-feature-settings: 'liga';
  -webkit-font-smoothing: antialiased;
}