You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2021/11/19 10:52:59 UTC

[GitHub] [airflow] torbjornvatn opened a new pull request #19702: Switch Markdown engine to markdown-it-py

torbjornvatn opened a new pull request #19702:
URL: https://github.com/apache/airflow/pull/19702


   As mentioned in #16435 will switching to markdown-it-py give much more advanced Markdown rendering and make it behave almost like to Github Flavored Markdown
   
   closes: #16435
   related:  #19644


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] torbjornvatn commented on pull request #19702: Switch Markdown engine to markdown-it-py

Posted by GitBox <gi...@apache.org>.
torbjornvatn commented on pull request #19702:
URL: https://github.com/apache/airflow/pull/19702#issuecomment-1019476980


   > You said above
   > 
   > > I would ideally find a way to remove the `display: block;` property from a `<summary>` inside a `<display>` to get a ‣ in front of the collapsible content just as here on Github.
   > 
   > I was just following that up.
   
   🤦🏼‍♂️ yes of course. Sorry for not remembering my own questions. 
   
   A CSS fix for this would be highly appreciated @bbovenzi The working ‣ for expanding the section is kind of important for the UX


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] bbovenzi edited a comment on pull request #19702: Switch Markdown engine to markdown-it-py

Posted by GitBox <gi...@apache.org>.
bbovenzi edited a comment on pull request #19702:
URL: https://github.com/apache/airflow/pull/19702#issuecomment-1061809069


   > A friendly reminder on this one @bbovenzi
   > 
   > I can do the CSS changes my self, I just needs some pointers on how
   
   My apologies. I kept missing this PR.
   
   We should be able to manipulate the css for markdown simply by adding something like
   ```
   .rich_doc summary {
     display: block;
   }
   ```
   to `main.css`


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] bbovenzi edited a comment on pull request #19702: Switch Markdown engine to markdown-it-py

Posted by GitBox <gi...@apache.org>.
bbovenzi edited a comment on pull request #19702:
URL: https://github.com/apache/airflow/pull/19702#issuecomment-1061809069


   > A friendly reminder on this one @bbovenzi
   > 
   > I can do the CSS changes my self, I just needs some pointers on how
   
   My apologies. I kept missing this PR.
   
   We should be able to manipulate the css for markdown simply by adding something like ```
   .rich_doc summary {
     display: block;
   }
   ``` to `main.css`


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] ashb commented on a change in pull request #19702: Switch Markdown engine to markdown-it-py

Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #19702:
URL: https://github.com/apache/airflow/pull/19702#discussion_r753183274



##########
File path: tests/www/test_utils.py
##########
@@ -184,29 +184,53 @@ def test_markdown_none(self):
 class TestWrappedMarkdown(unittest.TestCase):
     def test_wrapped_markdown_with_docstring_curly_braces(self):
         rendered = wrapped_markdown("{braces}", css_class="a_class")
-        assert '<div class="a_class" ><p>{braces}</p></div>' == rendered
+        self.assertEqual(

Review comment:
       Don't use asseryEqual, keep as a plain assert please.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] torbjornvatn commented on a change in pull request #19702: Switch Markdown engine to markdown-it-py

Posted by GitBox <gi...@apache.org>.
torbjornvatn commented on a change in pull request #19702:
URL: https://github.com/apache/airflow/pull/19702#discussion_r753281591



##########
File path: tests/www/test_utils.py
##########
@@ -184,29 +184,53 @@ def test_markdown_none(self):
 class TestWrappedMarkdown(unittest.TestCase):
     def test_wrapped_markdown_with_docstring_curly_braces(self):
         rendered = wrapped_markdown("{braces}", css_class="a_class")
-        assert '<div class="a_class" ><p>{braces}</p></div>' == rendered
+        self.assertEqual(

Review comment:
       @ashb fixed now




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] torbjornvatn commented on a change in pull request #19702: Switch Markdown engine to markdown-it-py

Posted by GitBox <gi...@apache.org>.
torbjornvatn commented on a change in pull request #19702:
URL: https://github.com/apache/airflow/pull/19702#discussion_r753207936



##########
File path: tests/www/test_utils.py
##########
@@ -184,29 +184,53 @@ def test_markdown_none(self):
 class TestWrappedMarkdown(unittest.TestCase):
     def test_wrapped_markdown_with_docstring_curly_braces(self):
         rendered = wrapped_markdown("{braces}", css_class="a_class")
-        assert '<div class="a_class" ><p>{braces}</p></div>' == rendered
+        self.assertEqual(

Review comment:
       Ok, will do
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] uranusjr commented on pull request #19702: Switch Markdown engine to markdown-it-py

Posted by GitBox <gi...@apache.org>.
uranusjr commented on pull request #19702:
URL: https://github.com/apache/airflow/pull/19702#issuecomment-1015245650


   So do we need to do anything special to `<summary>`’s `display: block`?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] torbjornvatn commented on pull request #19702: Switch Markdown engine to markdown-it-py

Posted by GitBox <gi...@apache.org>.
torbjornvatn commented on pull request #19702:
URL: https://github.com/apache/airflow/pull/19702#issuecomment-1014564403


   It was something else actually, fixed the conflict now
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] torbjornvatn commented on pull request #19702: Switch Markdown engine to markdown-it-py

Posted by GitBox <gi...@apache.org>.
torbjornvatn commented on pull request #19702:
URL: https://github.com/apache/airflow/pull/19702#issuecomment-1017795496


   > So do we need to do anything special to `<summary>`’s `display: block`?
   
   Are you asking me @uranusjr? I don't really know, do you want me to test it somehow?
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] torbjornvatn commented on a change in pull request #19702: Switch Markdown engine to markdown-it-py

Posted by GitBox <gi...@apache.org>.
torbjornvatn commented on a change in pull request #19702:
URL: https://github.com/apache/airflow/pull/19702#discussion_r823420942



##########
File path: airflow/www/static/css/main.css
##########
@@ -463,3 +463,7 @@ label[for="timezone-other"],
 .tooltip.d3-tip {
   z-index: 1070;
 }
+
+details summary {
+  display: list-item;

Review comment:
       👀  @bbovenzi 
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] torbjornvatn commented on a change in pull request #19702: Switch Markdown engine to markdown-it-py

Posted by GitBox <gi...@apache.org>.
torbjornvatn commented on a change in pull request #19702:
URL: https://github.com/apache/airflow/pull/19702#discussion_r825772732



##########
File path: tests/www/test_utils.py
##########
@@ -160,7 +160,7 @@ def setUp(self):
         self.attr_renderer = utils.get_attr_renderer()
 
     def test_python_callable(self):
-        def example_callable(unused_self):
+        def example_callable():

Review comment:
       I don't remember, putting it back now




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] torbjornvatn edited a comment on pull request #19702: Switch Markdown engine to markdown-it-py

Posted by GitBox <gi...@apache.org>.
torbjornvatn edited a comment on pull request #19702:
URL: https://github.com/apache/airflow/pull/19702#issuecomment-1014550984


   Seems like the conflict in `airflow/www/utils.py` is caused by someone else attempting to do something similar, but with a different Markdown lib than me. Will have to review that change to see if it's merged into master and makes this PR obsolete


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] uranusjr commented on pull request #19702: Switch Markdown engine to markdown-it-py

Posted by GitBox <gi...@apache.org>.
uranusjr commented on pull request #19702:
URL: https://github.com/apache/airflow/pull/19702#issuecomment-1013828168


   Any updates on this?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] github-actions[bot] commented on pull request #19702: Switch Markdown engine to markdown-it-py

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #19702:
URL: https://github.com/apache/airflow/pull/19702#issuecomment-1013779167


   This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] torbjornvatn commented on a change in pull request #19702: Switch Markdown engine to markdown-it-py

Posted by GitBox <gi...@apache.org>.
torbjornvatn commented on a change in pull request #19702:
URL: https://github.com/apache/airflow/pull/19702#discussion_r823420080



##########
File path: airflow/www/static/css/main.css
##########
@@ -463,3 +463,7 @@ label[for="timezone-other"],
 .tooltip.d3-tip {
   z-index: 1070;
 }
+
+details summary {
+  display: list-item;

Review comment:
       This trick works on both Firefox and Chrome. The ‣ is back 🎉 
   <img width="1114" alt="Screenshot 2022-03-10 at 08 29 36" src="https://user-images.githubusercontent.com/4453/157611503-1885c96b-faee-447c-a1c1-033e2d999f9f.png">
   <img width="1123" alt="Screenshot 2022-03-10 at 08 29 25" src="https://user-images.githubusercontent.com/4453/157611513-d3ef64f6-6e62-4ea6-b72e-a2a1f2106ca1.png">
   
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] torbjornvatn commented on a change in pull request #19702: Switch Markdown engine to markdown-it-py

Posted by GitBox <gi...@apache.org>.
torbjornvatn commented on a change in pull request #19702:
URL: https://github.com/apache/airflow/pull/19702#discussion_r823420080



##########
File path: airflow/www/static/css/main.css
##########
@@ -463,3 +463,7 @@ label[for="timezone-other"],
 .tooltip.d3-tip {
   z-index: 1070;
 }
+
+details summary {
+  display: list-item;

Review comment:
       This trick works on both Firefox and Chrome. The ‣ is back 🎉 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] bbovenzi commented on pull request #19702: Switch Markdown engine to markdown-it-py

Posted by GitBox <gi...@apache.org>.
bbovenzi commented on pull request #19702:
URL: https://github.com/apache/airflow/pull/19702#issuecomment-1061809069


   > A friendly reminder on this one @bbovenzi
   > 
   > I can do the CSS changes my self, I just needs some pointers on how
   
   My apologies. I kept missing this PR.
   
   We should be able to manipulate the css for markdown simply by adding something like ```.rich_doc summary {
     display: block;
   }
   ``` to `main.css`


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] torbjornvatn commented on pull request #19702: Switch Markdown engine to markdown-it-py

Posted by GitBox <gi...@apache.org>.
torbjornvatn commented on pull request #19702:
URL: https://github.com/apache/airflow/pull/19702#issuecomment-974062410


   @ferruzzi This one passed the Static checks


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] uranusjr commented on a change in pull request #19702: Switch Markdown engine to markdown-it-py

Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #19702:
URL: https://github.com/apache/airflow/pull/19702#discussion_r825387595



##########
File path: tests/www/test_utils.py
##########
@@ -160,7 +160,7 @@ def setUp(self):
         self.attr_renderer = utils.get_attr_renderer()
 
     def test_python_callable(self):
-        def example_callable(unused_self):
+        def example_callable():

Review comment:
       Why change this?

##########
File path: airflow/www/utils.py
##########
@@ -479,10 +479,11 @@ def json_render(obj, lexer):
 
 def wrapped_markdown(s, css_class='rich_doc'):
     """Convert a Markdown string to HTML."""
+    md = MarkdownIt("gfm-like")
     if s is None:
         return None
     s = textwrap.dedent(s)
-    return Markup(f'<div class="{css_class}" >' + markdown.markdown(s, extensions=['tables']) + "</div>")
+    return Markup(f'<div class="{css_class}" >' + md.render(s) + "</div>")

Review comment:
       ```suggestion
       return Markup(f'<div class="{css_class}" >{md.render(s)}</div>')
   ```




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] torbjornvatn commented on pull request #19702: Switch Markdown engine to markdown-it-py

Posted by GitBox <gi...@apache.org>.
torbjornvatn commented on pull request #19702:
URL: https://github.com/apache/airflow/pull/19702#issuecomment-983504945


   @ashb I would ideally find a way to remove the `display: block;` property from a `<summary>` inside a `<display>` to get a ‣ in front of the collapsible content just as here on Github. But I couldn't figure out how to change any of the CSS really 🤷🏼 
   
   Example:
   
   <details>
     <summary>Click to expand!</summary>
     
     ## Heading
     1. A numbered
     2. list
        * With some
        * Sub bullets
   </details>
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] torbjornvatn commented on pull request #19702: Switch Markdown engine to markdown-it-py

Posted by GitBox <gi...@apache.org>.
torbjornvatn commented on pull request #19702:
URL: https://github.com/apache/airflow/pull/19702#issuecomment-1014550984


   Seems like the conflict in `airflow/www/utils.py` is cause by someone else attempting to do something similar, but with a different Markdown lib than me


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] ashb commented on pull request #19702: Switch Markdown engine to markdown-it-py

Posted by GitBox <gi...@apache.org>.
ashb commented on pull request #19702:
URL: https://github.com/apache/airflow/pull/19702#issuecomment-1019140931


   @bbovenzi CSS questions - can you help out please


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] uranusjr commented on pull request #19702: Switch Markdown engine to markdown-it-py

Posted by GitBox <gi...@apache.org>.
uranusjr commented on pull request #19702:
URL: https://github.com/apache/airflow/pull/19702#issuecomment-1018165922


   You said above
   
   > I would ideally find a way to remove the `display: block;` property from a `<summary>` inside a `<display>` to get a ‣ in front of the collapsible content just as here on Github.
   
   I was just following that up.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] github-actions[bot] commented on pull request #19702: Switch Markdown engine to markdown-it-py

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #19702:
URL: https://github.com/apache/airflow/pull/19702#issuecomment-1064691616


   The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] torbjornvatn commented on pull request #19702: Switch Markdown engine to markdown-it-py

Posted by GitBox <gi...@apache.org>.
torbjornvatn commented on pull request #19702:
URL: https://github.com/apache/airflow/pull/19702#issuecomment-1061477538


   A friendly reminder on this one @bbovenzi 
   
   I can do the CSS changes my self, I just needs some pointers on how


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] torbjornvatn commented on pull request #19702: Switch Markdown engine to markdown-it-py

Posted by GitBox <gi...@apache.org>.
torbjornvatn commented on pull request #19702:
URL: https://github.com/apache/airflow/pull/19702#issuecomment-1066896743


   @uranusjr fixed the stuff you were asking about


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org