You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by GitBox <gi...@apache.org> on 2021/12/05 03:29:39 UTC

[GitHub] [calcite] guiyanakuang opened a new pull request #2632: [MINOR] Optimization of official website documentation

guiyanakuang opened a new pull request #2632:
URL: https://github.com/apache/calcite/pull/2632


   ## The purpose of this pr is to optimize the documentation on the official website
   Five separate files were modified
   
   **adapter.md**: Avoid code out of bounds, use scrollable code blocks instead 
   
   **algebra.md**: Added separate css styles for crowded tables to avoid tables going out of bounds and for better readability
   
   **reference.md**: As above, for table that are too wide, make them horizontally scrollable
   
   **tutorial.md**: Make the documentation consistent with the latest master example code
   
   **_style.scss**: New table css style definitions used
   
   ### Before and after comparison of some ui
   before
   ![image](https://user-images.githubusercontent.com/4069905/144732160-3983995d-005e-490a-aa22-ca9641c55b27.png)
   after
   ![image](https://user-images.githubusercontent.com/4069905/144732136-0e0277d5-6221-43a1-b1f3-8b721a16d25f.png)
   before
   ![image](https://user-images.githubusercontent.com/4069905/144732200-9e63ca16-3161-42ef-9518-1c95d931a0f3.png)
   after
   ![image](https://user-images.githubusercontent.com/4069905/144732256-9c751559-9bdb-4af5-be43-71d98acf651c.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@calcite.apache.org

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



[GitHub] [calcite] guiyanakuang commented on pull request #2632: [MINOR] Optimization of official website documentation

Posted by GitBox <gi...@apache.org>.
guiyanakuang commented on pull request #2632:
URL: https://github.com/apache/calcite/pull/2632#issuecomment-993234327


   @zabetak, I added a custom Jekyll hook plugin that will check before generating html to add a div parent to the table.  I feel good, if you have time to help check the ui effect. : )


-- 
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@calcite.apache.org

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



[GitHub] [calcite] zabetak commented on a change in pull request #2632: [MINOR] Optimization of official website documentation

Posted by GitBox <gi...@apache.org>.
zabetak commented on a change in pull request #2632:
URL: https://github.com/apache/calcite/pull/2632#discussion_r779547953



##########
File path: site/_plugins/wrap_table.rb
##########
@@ -0,0 +1,25 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to you under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+require 'nokogiri'
+
+Jekyll::Hooks.register [:pages, :documents], :post_render do |post|
+  if (post.is_a?(Jekyll::Document) && post.path.end_with?(".md")) ||
+    (post.is_a?(Jekyll::Page) && post.name.end_with?(".md"))

Review comment:
       I think this can be simplified to the following:
   ```suggestion
     if post.path.end_with?(".md")
   ```




-- 
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@calcite.apache.org

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



[GitHub] [calcite] zabetak commented on pull request #2632: [MINOR] Optimization of official website documentation

Posted by GitBox <gi...@apache.org>.
zabetak commented on pull request #2632:
URL: https://github.com/apache/calcite/pull/2632#issuecomment-1007366791


   Thanks for the PR @guiyanakuang , any improvement to the website is much appreciated ;)


-- 
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@calcite.apache.org

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



[GitHub] [calcite] guiyanakuang commented on pull request #2632: [MINOR] Optimization of official website documentation

Posted by GitBox <gi...@apache.org>.
guiyanakuang commented on pull request #2632:
URL: https://github.com/apache/calcite/pull/2632#issuecomment-989822777


   @zabetak, thanks for approving to this pr, I am reading the source code of calcite and look forward to the subsequent community collaboration.


-- 
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@calcite.apache.org

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



[GitHub] [calcite] zabetak commented on a change in pull request #2632: [MINOR] Optimization of official website documentation

Posted by GitBox <gi...@apache.org>.
zabetak commented on a change in pull request #2632:
URL: https://github.com/apache/calcite/pull/2632#discussion_r778192131



##########
File path: site/_docs/reference.md
##########
@@ -2630,9 +2630,9 @@ LIMIT 10;
 
 Result
 
-| c1     | c2    | c3      | c4      |
-| ------ | ----- | ------- | ------- |
-| OBJECT | ARRAY | INTEGER | BOOLEAN |
+|    c1    |   c2    |    c3     |    c4     |

Review comment:
       There are a lot of white-space changes in this file which do not seem necessary. It is better to avoid modifying lines if it is not needed to avoid cluttering the git history. If things are reasonably readable after adding `:` I would suggest to keep the rest as they were before.




-- 
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@calcite.apache.org

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



[GitHub] [calcite] guiyanakuang commented on pull request #2632: [MINOR] Optimization of official website documentation

Posted by GitBox <gi...@apache.org>.
guiyanakuang commented on pull request #2632:
URL: https://github.com/apache/calcite/pull/2632#issuecomment-993109505


   @zabetak, thanks for the tip, I got it, I'll see how to customize the behavior of the kramdown HTML converter later


-- 
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@calcite.apache.org

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



[GitHub] [calcite] zabetak commented on pull request #2632: [MINOR] Optimization of official website documentation

Posted by GitBox <gi...@apache.org>.
zabetak commented on pull request #2632:
URL: https://github.com/apache/calcite/pull/2632#issuecomment-989891958


   @guiyanakuang The changes about code wrapping and scrollability are a good idea but they are more useful if the apply everywhere. After playing a bit and resizing the window I found other tables that were overflowing. I pushed a few commits (https://github.com/apache/calcite/pull/2632/commits/53c974496b267ae329643fd0d995e87bcfdbf123, https://github.com/apache/calcite/pull/2632/commits/d7b2211b924c1f7286a4ff93b0380edf7246fe2c, https://github.com/apache/calcite/pull/2632/commits/e299b4b0516b9ddea8a7562bc48391232efc6e39) along these lines. What do you think?


-- 
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@calcite.apache.org

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



[GitHub] [calcite] guiyanakuang commented on pull request #2632: [MINOR] Optimization of official website documentation

Posted by GitBox <gi...@apache.org>.
guiyanakuang commented on pull request #2632:
URL: https://github.com/apache/calcite/pull/2632#issuecomment-1006666304


   Thanks for the reminder, I get it


-- 
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@calcite.apache.org

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



[GitHub] [calcite] zabetak commented on pull request #2632: [MINOR] Optimization of official website documentation

Posted by GitBox <gi...@apache.org>.
zabetak commented on pull request #2632:
URL: https://github.com/apache/calcite/pull/2632#issuecomment-1006612543


   General reminder: Please avoid rebasing when a review is in progress unless you are requested to do so. Rebasing makes new changes harder to see. In addition, when the reviewer has a local copy of the branch it is harder to incorporate the new changes if they have also performed small fixups locally.


-- 
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@calcite.apache.org

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



[GitHub] [calcite] vlsi commented on pull request #2632: [MINOR] Optimization of official website documentation

Posted by GitBox <gi...@apache.org>.
vlsi commented on pull request #2632:
URL: https://github.com/apache/calcite/pull/2632#issuecomment-992440461


   > Various improvements here (e.g., 14b2bd3) could already get merged (by me or other committers) without waiting the outcome of more complex parts
   
   Unfortunately, GitHub does not provide the UI for "stacked diffs" / "stacked pull requests" yet, so having and rebasing several pull requests "on top of each other" is time-consuming. If you want to have 5 dependent pull requests, and you patch one of them, it causes 5 consequent rebases.
   
   What I typically do is sort the commits in the order of confidence.
   In other words, trivial changes go first, then the more complicated. Then, "partial merge" is possible: you just advance `master` branch to the commit you want, and the rest is left in the PR diff as usual.
   
   


-- 
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@calcite.apache.org

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



[GitHub] [calcite] guiyanakuang commented on a change in pull request #2632: [MINOR] Optimization of official website documentation

Posted by GitBox <gi...@apache.org>.
guiyanakuang commented on a change in pull request #2632:
URL: https://github.com/apache/calcite/pull/2632#discussion_r778539719



##########
File path: site/_docs/reference.md
##########
@@ -2630,9 +2630,9 @@ LIMIT 10;
 
 Result
 
-| c1     | c2    | c3      | c4      |
-| ------ | ----- | ------- | ------- |
-| OBJECT | ARRAY | INTEGER | BOOLEAN |
+|    c1    |   c2    |    c3     |    c4     |

Review comment:
       I agree with your comments and finished the changes.




-- 
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@calcite.apache.org

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



[GitHub] [calcite] zabetak commented on pull request #2632: [MINOR] Optimization of official website documentation

Posted by GitBox <gi...@apache.org>.
zabetak commented on pull request #2632:
URL: https://github.com/apache/calcite/pull/2632#issuecomment-992416576


   @guiyanakuang also a small tip: In the future try to keep PRs self contained so that they can get merged easier. Various improvements here (e.g., https://github.com/apache/calcite/commit/14b2bd388aa6400af110d560422badd09bc4f4d5) could already get merged (by me or other committers) without waiting the outcome of more complex parts such as the "scrollable" tables.


-- 
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@calcite.apache.org

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



[GitHub] [calcite] guiyanakuang commented on pull request #2632: [MINOR] Optimization of official website documentation

Posted by GitBox <gi...@apache.org>.
guiyanakuang commented on pull request #2632:
URL: https://github.com/apache/calcite/pull/2632#issuecomment-989976407


   > @guiyanakuang The changes about code wrapping and scrollability are a good idea but they are more useful if the apply everywhere. After playing a bit and resizing the window I found other tables that were overflowing. I pushed a few commits ([53c9744](https://github.com/apache/calcite/commit/53c974496b267ae329643fd0d995e87bcfdbf123), [d7b2211](https://github.com/apache/calcite/commit/d7b2211b924c1f7286a4ff93b0380edf7246fe2c), [e299b4b](https://github.com/apache/calcite/commit/e299b4b0516b9ddea8a7562bc48391232efc6e39)) along these lines. What do you think?
   
   My thinking when I committed this pr was to address the most crowded tables and to ensure that changes were minimised. Indeed the rest of the tables may overflow even after adjusting the width. If you don't mind a line break in the code, I'd agree to modify the css of the table.  `overflow-x: auto;`  looks better.


-- 
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@calcite.apache.org

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



[GitHub] [calcite] zabetak commented on a change in pull request #2632: [MINOR] Optimization of official website documentation

Posted by GitBox <gi...@apache.org>.
zabetak commented on a change in pull request #2632:
URL: https://github.com/apache/calcite/pull/2632#discussion_r779561508



##########
File path: site/_plugins/wrap_table.rb
##########
@@ -0,0 +1,25 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to you under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+require 'nokogiri'
+
+Jekyll::Hooks.register [:pages, :posts, :documents], :post_render do |post|
+    match = post.content.match /^<table>$/
+    if match

Review comment:
       Thanks for revisiting this part. I left another minor comment to the new version of the code but other than that I think the PR is in very good shape.




-- 
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@calcite.apache.org

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



[GitHub] [calcite] zabetak closed pull request #2632: [MINOR] Optimization of official website documentation

Posted by GitBox <gi...@apache.org>.
zabetak closed pull request #2632:
URL: https://github.com/apache/calcite/pull/2632


   


-- 
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@calcite.apache.org

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



[GitHub] [calcite] zabetak commented on pull request #2632: [MINOR] Optimization of official website documentation

Posted by GitBox <gi...@apache.org>.
zabetak commented on pull request #2632:
URL: https://github.com/apache/calcite/pull/2632#issuecomment-1007366791


   Thanks for the PR @guiyanakuang , any improvement to the website is much appreciated ;)


-- 
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@calcite.apache.org

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



[GitHub] [calcite] zabetak closed pull request #2632: [MINOR] Optimization of official website documentation

Posted by GitBox <gi...@apache.org>.
zabetak closed pull request #2632:
URL: https://github.com/apache/calcite/pull/2632


   


-- 
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@calcite.apache.org

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



[GitHub] [calcite] zabetak commented on pull request #2632: [MINOR] Optimization of official website documentation

Posted by GitBox <gi...@apache.org>.
zabetak commented on pull request #2632:
URL: https://github.com/apache/calcite/pull/2632#issuecomment-992409154


   > I think it is better to wrap a special div in the outer layer to deal with this feature of horizontal scrolling.
   
   @guiyanakuang You 're probably right I think the best would be to wrap all tables in a div. However, I would prefer if we could do this without modifying every single .md file. It seems that there is a way to do this by customizing the behavior of the kramdown HTML converter (see discussion under https://github.com/gettalong/kramdown/issues/69).
   


-- 
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@calcite.apache.org

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



[GitHub] [calcite] zabetak commented on pull request #2632: [MINOR] Optimization of official website documentation

Posted by GitBox <gi...@apache.org>.
zabetak commented on pull request #2632:
URL: https://github.com/apache/calcite/pull/2632#issuecomment-998217167


   @guiyanakuang sure I will make another pass in the following days.


-- 
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@calcite.apache.org

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



[GitHub] [calcite] guiyanakuang commented on a change in pull request #2632: [MINOR] Optimization of official website documentation

Posted by GitBox <gi...@apache.org>.
guiyanakuang commented on a change in pull request #2632:
URL: https://github.com/apache/calcite/pull/2632#discussion_r779611953



##########
File path: site/_plugins/wrap_table.rb
##########
@@ -0,0 +1,25 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to you under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+require 'nokogiri'
+
+Jekyll::Hooks.register [:pages, :documents], :post_render do |post|
+  if (post.is_a?(Jekyll::Document) && post.path.end_with?(".md")) ||
+    (post.is_a?(Jekyll::Page) && post.name.end_with?(".md"))

Review comment:
       LGTM
   
   




-- 
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@calcite.apache.org

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



[GitHub] [calcite] guiyanakuang edited a comment on pull request #2632: [MINOR] Optimization of official website documentation

Posted by GitBox <gi...@apache.org>.
guiyanakuang edited a comment on pull request #2632:
URL: https://github.com/apache/calcite/pull/2632#issuecomment-989976407


   > @guiyanakuang The changes about code wrapping and scrollability are a good idea but they are more useful if the apply everywhere. After playing a bit and resizing the window I found other tables that were overflowing. I pushed a few commits ([53c9744](https://github.com/apache/calcite/commit/53c974496b267ae329643fd0d995e87bcfdbf123), [d7b2211](https://github.com/apache/calcite/commit/d7b2211b924c1f7286a4ff93b0380edf7246fe2c), [e299b4b](https://github.com/apache/calcite/commit/e299b4b0516b9ddea8a7562bc48391232efc6e39)) along these lines. What do you think?
   
   My thinking when I committed this pr was to address the most crowded tables and to ensure that changes were minimised. Indeed the rest of the tables may overflow even after adjusting the width. If you don't mind a line break in the code, I'd agree to modify the css of the table.  `overflow-x: auto;`  looks better.
   
   If you don't mind, I'll use my work computer tomorrow to verify the results, I'm not comfortable compiling jekyll right 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@calcite.apache.org

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



[GitHub] [calcite] zabetak commented on pull request #2632: [MINOR] Optimization of official website documentation

Posted by GitBox <gi...@apache.org>.
zabetak commented on pull request #2632:
URL: https://github.com/apache/calcite/pull/2632#issuecomment-990008705


   How "crowded" a table is does not depend only on the content but also on the size of the screen/window/browser.
   
   I did verify that changes seem to have the intended outcome but it would be be good if you can double-check in case I missed something. Looking forward for your feedback.


-- 
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@calcite.apache.org

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



[GitHub] [calcite] guiyanakuang commented on a change in pull request #2632: [MINOR] Optimization of official website documentation

Posted by GitBox <gi...@apache.org>.
guiyanakuang commented on a change in pull request #2632:
URL: https://github.com/apache/calcite/pull/2632#discussion_r778539372



##########
File path: site/_plugins/wrap_table.rb
##########
@@ -0,0 +1,25 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to you under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+require 'nokogiri'
+
+Jekyll::Hooks.register [:pages, :posts, :documents], :post_render do |post|
+    match = post.content.match /^<table>$/
+    if match

Review comment:
       I meant to make sure that the processed post.content is a valid html target, `doc.search(table)` may cause side effects if processed on a css file content.
   
   I did a deeper investigation, refactored the implementation, and now narrow the scope of processing by making sure the file names processed are suffixed with .md to avoid side effects, and I removed the Hook for `Posts`, which is a subset of `Documents` I removed to avoid duplicate processing.




-- 
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@calcite.apache.org

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



[GitHub] [calcite] zabetak commented on a change in pull request #2632: [MINOR] Optimization of official website documentation

Posted by GitBox <gi...@apache.org>.
zabetak commented on a change in pull request #2632:
URL: https://github.com/apache/calcite/pull/2632#discussion_r778159958



##########
File path: site/_plugins/wrap_table.rb
##########
@@ -0,0 +1,25 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to you under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+require 'nokogiri'
+
+Jekyll::Hooks.register [:pages, :posts, :documents], :post_render do |post|
+    match = post.content.match /^<table>$/
+    if match

Review comment:
       @guiyanakuang Do we really need this check? Isn't the `doc.search("table")` below enough for this purpose? Do we gain something in performance with 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@calcite.apache.org

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



[GitHub] [calcite] guiyanakuang commented on pull request #2632: [MINOR] Optimization of official website documentation

Posted by GitBox <gi...@apache.org>.
guiyanakuang commented on pull request #2632:
URL: https://github.com/apache/calcite/pull/2632#issuecomment-997816971


   @zabetak I feel the pr is ready, can you review it again.


-- 
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@calcite.apache.org

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