[GitHub] zeppelin issue #1384: [ZEPPELIN-1387] Support table syntax in markdown inter...
Github user corneadoug commented on the issue: https://github.com/apache/zeppelin/pull/1384 Awesome, Merging if there is no more discussions --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1384: [ZEPPELIN-1387] Support table syntax in markdown inter...
Github user 1ambda commented on the issue: https://github.com/apache/zeppelin/pull/1384 Finally, CI is green! :) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1384: [ZEPPELIN-1387] Support table syntax in markdown inter...
Github user corneadoug commented on the issue: https://github.com/apache/zeppelin/pull/1384 Usually we prefer rebase since it doesn't pollute the commit history. But its fine, the end result is the same. We sometimes have flaky tests, I will check once the build is finished --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1384: [ZEPPELIN-1387] Support table syntax in markdown inter...
Github user 1ambda commented on the issue: https://github.com/apache/zeppelin/pull/1384 Open PR to re-trigger CI --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1384: [ZEPPELIN-1387] Support table syntax in markdown inter...
Github user 1ambda commented on the issue: https://github.com/apache/zeppelin/pull/1384 Close PR to re-trigger CI --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1384: [ZEPPELIN-1387] Support table syntax in markdown inter...
Github user 1ambda commented on the issue: https://github.com/apache/zeppelin/pull/1384 Now CI is failing on the third build job. :( I checked out `zeppelin/master` and merged it on `1ambda/fix-zeppelin-1387`. Should I have use git `rebase` command instead? I thought using `rebase` command will overwrite this PR history (comments, etc...) https://cloud.githubusercontent.com/assets/4968473/18695206/2afb0c0c-7feb-11e6-8d94-119a456876a0.png;> https://cloud.githubusercontent.com/assets/4968473/18695207/2cd2ce8e-7feb-11e6-961d-5292d58513cf.png;> --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1384: [ZEPPELIN-1387] Support table syntax in markdown inter...
Github user corneadoug commented on the issue: https://github.com/apache/zeppelin/pull/1384 That CI failure is being handled in #1444, you should be able to rebase and have a green CI soon --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1384: [ZEPPELIN-1387] Support table syntax in markdown inter...
Github user AhyoungRyu commented on the issue: https://github.com/apache/zeppelin/pull/1384 @1ambda Yeah tested locally and looks good. Thanks for your quick response! Now it's really ready to be merged I think ð The other PRs' CI are failing in Selenium test because of the same reason. Currently it's tracked in [ZEPPELIN-1449](https://issues.apache.org/jira/browse/ZEPPELIN-1449). --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1384: [ZEPPELIN-1387] Support table syntax in markdown inter...
Github user 1ambda commented on the issue: https://github.com/apache/zeppelin/pull/1384 https://cloud.githubusercontent.com/assets/4968473/18679870/5ae08dec-7f9c-11e6-8c61-91e87bad5ed9.png;> CI is failing on only the last job. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1384: [ZEPPELIN-1387] Support table syntax in markdown inter...
Github user 1ambda commented on the issue: https://github.com/apache/zeppelin/pull/1384 @AhyoungRyu I'v resolved the merge conflict with 8f344db. Could you try again? :) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1384: [ZEPPELIN-1387] Support table syntax in markdown inter...
Github user AhyoungRyu commented on the issue: https://github.com/apache/zeppelin/pull/1384 @1ambda Sadly the conflict needs to be resolved first before merging. But only for `docs/interpreter/markdown.md` this file :) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1384: [ZEPPELIN-1387] Support table syntax in markdown inter...
Github user Leemoonsoo commented on the issue: https://github.com/apache/zeppelin/pull/1384 Thanks @1ambda. Looks great to me. Merge if there're no further discussions. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1384: [ZEPPELIN-1387] Support table syntax in markdown inter...
Github user 1ambda commented on the issue: https://github.com/apache/zeppelin/pull/1384 @Leemoonsoo I'v fixed license description. Also, now I understand when to describe licenses to `licenses` or `bin_licenses`. Thanks. :) @bzz I think it is better to use more specific exception types in catch block as you pointed out. Could you review e08929a again? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1384: [ZEPPELIN-1387] Support table syntax in markdown inter...
Github user bzz commented on the issue: https://github.com/apache/zeppelin/pull/1384 @1ambda ð fot tests, looks great to me except for licensing issue raised by Moon and small comment above. If you could let me know you user name in ASF JIRA I'll be happy to help assigning ZEPPELIN-1387 to you. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1384: [ZEPPELIN-1387] Support table syntax in markdown inter...
Github user Leemoonsoo commented on the issue: https://github.com/apache/zeppelin/pull/1384 Because now github-markdown-css is downloaded on build time instead of packed in the source tree, related license now need to moved back to under bin_licenses. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1384: [ZEPPELIN-1387] Support table syntax in markdown inter...
Github user felixcheung commented on the issue: https://github.com/apache/zeppelin/pull/1384 looks fine to me, I'd defer to others on licensing. thanks for the great contribution! --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1384: [ZEPPELIN-1387] Support table syntax in markdown inter...
Github user corneadoug commented on the issue: https://github.com/apache/zeppelin/pull/1384 @Leemoonsoo @felixcheung Are we good on that PR? CI is failing on tests, not related to this PR as its happening to other PRs. But would be better to merge this one after CI is fixed --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1384: [ZEPPELIN-1387] Support table syntax in markdown inter...
Github user corneadoug commented on the issue: https://github.com/apache/zeppelin/pull/1384 @1ambda Usually to include external source css, we would use the `assets/styles` folder. However in this case, you can use bower to have it part of the build instead of the source code, by using `bower install --save github-markdown-css` --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1384: [ZEPPELIN-1387] Support table syntax in markdown inter...
Github user AhyoungRyu commented on the issue: https://github.com/apache/zeppelin/pull/1384 @1ambda Looks good! ðð for adding screenshot images. >It seems that description does't show even though i provide it to markdown/src/main/resources/interpreter-setting.json like yeah I also felt uncomfortable when create/edit a interpreter. There is no way to get information what these properties are working for. It would be better to add `description` column separately to the interpreter setting table I think. But it can be handled in the different PR :) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1384: [ZEPPELIN-1387] Support table syntax in markdown inter...
Github user 1ambda commented on the issue: https://github.com/apache/zeppelin/pull/1384 @AhyoungRyu Thanks for for your review and comments :) I'v updated docs and added missing license declaration in some java files. Now docs/markdown looks like https://cloud.githubusercontent.com/assets/4968473/18229076/06d5b834-72a5-11e6-994c-585def006adc.png;> https://cloud.githubusercontent.com/assets/4968473/18229096/80ba7a22-72a5-11e6-8c8c-46a105e2bf96.png;> https://cloud.githubusercontent.com/assets/4968473/18229083/0b7b6bcc-72a5-11e6-9688-c1f0238634c4.png;> --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1384: [ZEPPELIN-1387] Support table syntax in markdown inter...
Github user AhyoungRyu commented on the issue: https://github.com/apache/zeppelin/pull/1384 @1ambda Great! It works well as expected ð If you want to add the interpreter property(`markdown.parser.type`) to the markdown interpreter, it would be better to provide that information via [Zeppelin markdown interpreter docs](https://zeppelin.apache.org/docs/0.7.0-SNAPSHOT/interpreter/markdown.html) with configuration table as like the other interpreters do. Nothing much needed. You can just add the below conf table to `markdown.md`. ``` ## Configuration Name Default Value Description markdown.parser.type markdown4j Markdown Parser Type. Available values: markdown4j, pegdown. ``` Also the below sentences in [Overview section](https://zeppelin.apache.org/docs/0.7.0-SNAPSHOT/interpreter/markdown.html#overview) need to be updated accordingly. >Markdown is a plain text formatting syntax designed so that it can be converted to HTML. Apache Zeppelin uses markdown4j. For more examples and extension support, please checkout here. FYI, `markdown.md` file is under `ZEPPELIN_HOME/docs/interpreter/` :) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1384: [ZEPPELIN-1387] Support table syntax in markdown inter...
Github user 1ambda commented on the issue:
https://github.com/apache/zeppelin/pull/1384
I'v restored markdown4j parser and add `markdown.parser.type` property to
`md` group. Pegdown parser will be used only if user specify `pegdown` property
value.
https://cloud.githubusercontent.com/assets/4968473/18211602/66b9cf9c-7179-11e6-8506-9ae6fcfbfe3b.png;>
One more question. It seems that `description` does't show even if i
already provide in `markdown/src/main/resources/interpreter-setting.json` like
```json
[
{
"group": "md",
"name": "md",
"className": "org.apache.zeppelin.markdown.MarkdownInterpreter",
"properties": {
"markdown.parser.type": {
"envName": "MARKDOWN_PARSER_TYPE",
"propertyName": "markdown.parser.type",
"defaultValue": "markdown4j",
"description": "Markdown Parser Type. Available values: markdown4j,
pegdown. Default = markdown4j"
}
}
}
]
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin issue #1384: [ZEPPELIN-1387] Support table syntax in markdown inter...
Github user 1ambda commented on the issue: https://github.com/apache/zeppelin/pull/1384 @Leemoonsoo I think **option 2** (Adding a property) is a better solution. because 1) Adding a new interpreter just for support another markdown parser might be confusing to users 2) Pegdown parser will be able to replace markdown4j parser soon if we implement YUML and Websequence. At that time, we can just modify the parser property of `md` interpreter instead of requesting for users to change their interpreter type --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1384: [ZEPPELIN-1387] Support table syntax in markdown inter...
Github user AhyoungRyu commented on the issue: https://github.com/apache/zeppelin/pull/1384 @1ambda Yeah tested again and it looks goodð Thanks! (One more thing: [Zeppelin's Java block indentation size](https://google.github.io/styleguide/javaguide.html#s4.2-block-indentation) is `2 spaces` :D ) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1384: [ZEPPELIN-1387] Support table syntax in markdown inter...
Github user Leemoonsoo commented on the issue: https://github.com/apache/zeppelin/pull/1384 Regarding @felixcheung's comment, I think keeping backward compatibility is always very important. If i add more possible options after @1ambda 's - Create new interpreter %pd that uses pegdown and restore %md to use markdown4j - Add a interpreter property that selects markdown library pegdown/markdown4j. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1384: [ZEPPELIN-1387] Support table syntax in markdown inter...
Github user 1ambda commented on the issue: https://github.com/apache/zeppelin/pull/1384 @AhyoungRyu It was due to the `ANCHORLINK` option in pegdown. I removed it.  --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1384: [ZEPPELIN-1387] Support table syntax in markdown inter...
Github user AhyoungRyu commented on the issue: https://github.com/apache/zeppelin/pull/1384 @1ambda Yeah it's good idea to apply github markdown style to Zeppelin. I tested again and it works well. Definitely better than before! But I found little bit wired behavior when I made a title. ``` ## This is second level title ### This is third level title ``` This title code makes a link and it's connected to Zeppelin home. Is it intended?  --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1384: [ZEPPELIN-1387] Support table syntax in markdown inter...
Github user felixcheung commented on the issue: https://github.com/apache/zeppelin/pull/1384 Personally YUML and Websequence is very handy - I use them quite a bit. What do other think about this? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1384: [ZEPPELIN-1387] Support table syntax in markdown inter...
Github user 1ambda commented on the issue: https://github.com/apache/zeppelin/pull/1384 Now markdown table looks like https://cloud.githubusercontent.com/assets/4968473/18141148/54414ab6-6ff3-11e6-897d-94dab08e6516.png;> --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1384: [ZEPPELIN-1387] Support table syntax in markdown inter...
Github user felixcheung commented on the issue: https://github.com/apache/zeppelin/pull/1384 MArkdown4j supports YUML and Websequence, does this support the same? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1384: [ZEPPELIN-1387] Support table syntax in markdown inter...
Github user 1ambda commented on the issue: https://github.com/apache/zeppelin/pull/1384 @Leemoonsoo Thanks! I'v added missing transitive deps and removed unused markdown4j dep. I also found that `org.ow2.asm:asm:jar:3.0.1` is described in `zeppelin-distribution/src/licence_bin/LICENSE` But already denoted license is different version (3.0.1) compared to pegdown's dependency (5.0.3). Should I updated `org.ow2.asm:asm:jar:3.0.1` to `org.ow2.asm:asm:jar:5.0.3`? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1384: [ZEPPELIN-1387] Support table syntax in markdown inter...
Github user bzz commented on the issue: https://github.com/apache/zeppelin/pull/1384 The change of library looks great to me, ð for having a test case! Few things to take care of, before merging it: - for all the code, please make sure it follows project [styleguide\code conventions](https://github.com/apache/zeppelin/blob/master/CONTRIBUTING.md#code-convention) - as you add a new dependency - it will become a part of the next release convenience binary for Zeppelin and in order to be included, we need to verify it's licence compatibility. Can you please add it to `zeppelin-distribution/src/licence_bin/LICENSE` - also, applying some styles for such tables, as noted by @AhyoungRyu sounds like a good improvement as well --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1384: [ZEPPELIN-1387] Support table syntax in markdown inter...
Github user AhyoungRyu commented on the issue: https://github.com/apache/zeppelin/pull/1384 @1ambda Thank you for the contribution! I just shortly look through the screenshot in the description, but comparing to Github markdown support it seems Zeppelin's table doesn't have a table border. If I make the below table in Github, ``` | Tables | Are | Cool | |--|:-:|--:| | col 1 is | left-aligned | $1600 | | col 2 is |centered | $12 | | col 3 is | right-aligned |$1 | ``` it will be shown like this. https://cloud.githubusercontent.com/assets/10060731/18073629/8b893116-6ea2-11e6-9166-46e246acbc04.png;> But in Zeppelin it doesn't have the border. https://cloud.githubusercontent.com/assets/10060731/18073791/7681b1ca-6ea3-11e6-9229-3104a2dc.png;> --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1384: [ZEPPELIN-1387] Support table syntax in markdown inter...
Github user bzz commented on the issue: https://github.com/apache/zeppelin/pull/1384 @1ambda thank you for improvement! Let me review and get back to you with it. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
