Markdown linter integration #49
Assignees
Labels
Comments
|
@crandmck pointed me to me this ticket as something I might be able to help with. What's the specific goal/deliverable here? Is it: Create a script that reads & validates (lints) the top level |
|
@Sequoia Start with loopback.io files. We can decide after that trial run to see if we want to integrate the script into each LB repo afterwards based on your discoveries (and standards you decide upon in the configuration of remark-lint -- we can discuss more during the review of your PR to loopback.io -- cc me in that PR). |
|
@Sequoia Please reference your PR to this issue too. |
|
To clarify: the point of this exercise was to detect and resolve layout errors in loopback.io due to poorly-formatted markdown. Very little of that exists in the "regular" markdown files in Far more useful would be to correct problems in the READMEs being pulled into the docs from other repos (repos.json lists them). For example: http://loopback.io/doc/en/lb2/Tutorial-android-getting-started.html (list at end of document does not layout correctly) and http://loopback.io/doc/en/lb2/Tutorial-access-control.html (headings not formatted correctly). |
|
mdl uses Kramdown (and looks like it could do the job). Noting that here in case there are issues with remark-lint (which uses remark). Moving forward with remark-lint because while this repo does already depend on ruby, this particular task (extra-repo READMEs) already uses JS (get-readmes). |
|
Starting point: linting _includes/readmes:
remark ./_includes/readmes 745 warningsThese are primarily |
|
Still working on this... I had to do a bit of a deep-dive to figure out why remark-lint was not catching any of the faulty headings (I wasn't sure if I was configuring it wrong or what). Turns out the issue is a bit trickier: because
|
|
OK, great...please do see if you can find a tool that detects the header issue (missing space after |
|
Should also try to see if the tool you're trying out supports a |
Fenced code blocks in nested listsThis one is very tricky... 7 Space indentOL > UL1. Top level
* Second level
```
<android-sdk>/extras/google/google_play_services/libproject/google-play-services_lib/
```
* Another on second level
UL > UL* Top level
* Second level
```
<android-sdk>/extras/google/google_play_services/libproject/google-play-services_lib/
```
* Another on second level
4 spaces indentOL > UL1. Top level
* Second level
```
<android-sdk>/extras/google/google_play_services/libproject/google-play-services_lib/
```
* Another on second level
UL > UL* Top level
* Second level
```
<android-sdk>/extras/google/google_play_services/libproject/google-play-services_lib/
```
* Another on second level
leading/trailing newlines
Conclusions
I'm not sure how to make it work across all the repos besides simply telling people to learn these rules or avoid fenced blocks in (nested) lists altogether. |
|
I'm still working on finding a combination of rules that would detect this issue. |
|
I do not believe there's any combination of rules that will detect the fenced code block issue (using markdownlint which I'm using because it can detect the |
Newline missing before list itemsThis was present in the loopback-android-getting-started README: 1. Import Google Play services library project into your workspace. The
project is located inside the directory where you have installed i
the Android SDK.
* Run File > Import
* Select Android > Existing Android Code Into Workspace
* Go to
```
<android-sdk>/extras/google/google_play_services/libproject/google-play-services_lib/
```
* Select google-play-services_lib for import
* Check "Copy projects into workspace"
* Click "Finish".
See this page for more details:
[Set Up Google Play Services SDK](http://developer.android.com/google/play-services/setup.html)
1. Add google-play-services\_lib as a build dependency of the Guide Application
* In the Package Explorer, select LoopbackGuideApplication
* Run File > Project Properties
* Select Android
* In the Library frame, click on "Add..." and select google-play-services_libThat bit in the middle "See this page for more details" breaks the list rendering starting with "Add google-play-services...", even if the ThoughtsComplex listsI'm not sure what the intention was here: was "See this page" supposed to be part of "Click 'finish'" list item, or "Import Google Play" list item?? Or is the following Authors must avoid complex nested list structures when creating markdown files we wish to use Kramdown. Linting markdown in generalMarkdown's strength, that it's terse, forgiving, implicit & easy to write, is also its weakness here. The following is not "invalid" markdown: this will stop the list on the next line from rendering properly
1. This is just some text as far as markdown is concerned
2. This is more text that will display, all on one line with the aboveIt is valid markdown that renders the following:
Note that github markdown parser creates a new Basically, there's very little that qualifies as "invalid markdown," just "unrecognized markdown." It's a bit of a Catch 22 because either
Conclusions
To rephrase, linting cannot reliably catch all issues and it will report false-positives (wrt. rendering-- if we enforce a strict style the second issue is moot).
RecommendationWhile linting isn't guaranteed to catch issues, a 100% lint-free file is less likely to have them.
|
Working versionFor the record, making that list completely lint-free did cause it to render as expected (with the major # loopback-android-getting-started
LoopBack mobile application to show developers how to use
LoopBack Open Node.js Mobile API Middle tier.
## Setting up the Development Environment
1. Install [Eclipse Android Development Tools](http://developer.android.com/sdk/index.html)
1. Open Window > Android SDK Manager, make sure you have these modules
installed:
* Tools > Android SDK Platform-tools 18 or newer
* Tools > Android SDK Build-tools 18 or newer
* Android 4.3 (API 18) > SDK Platform
* Extras > Google Play Services
1. Import the Android project in this directory:
* Run File > Import
* Select Android > Existing Android Code Into Workspace
* Choose the directory where you have the Getting Started application.
* Select LoopBackExample for import
1. Import Google Play services library project into your workspace.
The project is located inside the directory where you have installed
the Android SDK.
* Run File > Import
* Select Android > Existing Android Code Into Workspace
* Go to
```xml
<android-sdk>/extras/google/google_play_services/libproject/google-play-services_lib/
```
* Select google-play-services_lib for import
* Check "Copy projects into workspace"
* Click "Finish".
See this page for more details:
[Set Up Google Play Services SDK](http://developer.android.com/google/play-services/setup.html)
1. Add google-play-services\_lib as a build dependency of the Guide Application
* In the Package Explorer, select LoopbackGuideApplication
* Run File > Project Properties
* Select Android
* In the Library frame, click on "Add..." and select google-play-services_lib
1. Obtain an API key for Google Maps Android API v2
[instructions](https://developers.google.com/maps/documentation/android/start#obtaining_an_api_key)
and enter it into AndroidManifest.xml.
---
[More LoopBack examples](https://github.com/strongloop/loopback-example)So perhaps the organization should enforce markdown style rules across all repos: while it would be more work for authors, it is likely to reduce parsing issues. |
|
Thanks for chasing this all down @Sequoia. Your findings mostly align with my experience, in that there are lots of wrinkles to how GH/Jekyll handles markdown, especially around lists. In general, I do think we need to be able to use code blocks in lists (ordered and unordered), but I also think it's fine if we have to fix these manually.
That's fine by me, and I think it's the most realistic scenario. Not sure how @superkhau feels.
It depends on what you mean by "fancy", but this is probably OK. There are often other ways to work around it, too (put code blocks in a separate section and link to it, et. al.) In general, in the loopback.io doc content (i.e. md files in |
|
@crandmck @Sequoia I feel we need to cater for GitHub based workflows and not Jekyll. The contributors all know GHFM. Maybe there is a GHFM->Kramdown converter? Spike on more libs?
This way, no changes are required to any dev workflows and you guys can have the looks you want for public consumption.
I would prefer that we do not have to go down this route (don't want to make all devs learn yet another format on top of the standard GHFM -- this includes all the explaining we have to do during reviews). |
|
Summary: Use Github’s markdown parser so behavior is consistent from GH to Jeyll. Thanks for the info @superkhau! With those priorities in mind, here are my thoughts: Markdown has no formal standard, so each parser has its own de-facto standard style. It’s best therefore to treat the parser as the standard. Building a tool to translate or transform one style to another would, in my opinion, be a significant undertaking and probably not worth the effort. It would be like writing ZSH scripts then writing a script to translate them to BASH: most people just “pick one [shell] and stick with it.” That’s what I think makes the most sense here: pick one [parser] and use it in all places. This line of thinking leads to the conclusion that we should change our Jekyll site to match what Github does, as we can’t do the opposite. Specifically, Github uses markup to parse markdown, where we use kramdown. I am researching using “markup” with jekyll now, I imagine it’s possible. This solution may cause some issues with existing pages on the site. Switching to GH/markup might necessitate updating some existing pages on our Jekyll site. The way I see it we’re going to have to update our style in one place or another, so this may just be necessary. I'm going to look into the viability of this solution. Failing this, I can just document the known quirks per step 2 above (this would be a "write them down as you find them" process). Let me know if you'd like me to look into a scripted solution. Additional NotesSide note: One of the original issues (headings) is fixed by commenting out |
|
Thanks again Sequoia for looking into this... If the diffs between kramdown and GFM are causing an issue, I think we could configure kramdown to parse markdown as GFM by using the However, before we spend any more time on this, I think it's important to remember the purpose of this exercise (in my view anyway): to ensure that the markdown files that we "fetch" from other repos layout properly in the docs as well as on GitHub and npm. At this point, it would take probably a total of a couple of hours to just manually edit all the READMEs in question and ensure they work correctly everywhere. Yes, having an automated process for this would be nice, but if requires a lot more effort or requires us to make significant changes to other parts of the docs, I don't think it's worth it. Markdown is not code... and if there are minor diffs that are not visible to the viewer, it doesn't really matter IMHO. |
|
@crandmck I'm happy to do that checking/editing, I can start right away! I hadn't considered that because I'd taken from this discussion that "requiring devs on different repos to change their readmes is a no-go." I can go ahead and do that editing & start sending PRs. A couple more observations/thoughts:
Possible long-term solution
|
|
For now I'll work on sending PRs to offending repos. |
|
OK, thanks @Sequoia. Certainly cleaning up READMEs manully is NOT a "no go"... I just think @superkhau rightly was looking for a way to automate it. My point is just that it's relatively straightforward (the number of files and corrections is not that huge), so perhaps the easiest way to achieve our high-level goal is to "just do it" and not spend a lot of time on the intricacies of markdown parsers and linting.
IMHO this would be a problem, because we have a ton of "include" templates (note, warning, etc) that use |
|
@crandmck @Sequoia Let's agree on the vision before deciding on the solutions:
We should create a document in KB with those mission statements and I think @Sequoia should add to that document with some proposals with pros/cons of each then we can decide on which route to go with a short hangout. |
|
@superkhau That sounds good. One key point: It's not a 100% solution, but in many of the cases I've seen, adhering strictly to the rules set out in the original markdown "spec" eliminates most issues. For example, github permits indenting sub-lists with 2 spaces but if you use 4 per the spec* the problems go away. It's hard to target Redcarpet ("GFM") as "first class" because it is the most permissive of the parsers in our system, so what it accepts frequently does work the same for Kramdown. Possible approaches
My recommendation for immediate next stepsWhile I have not found a silver bullet solution and presumably more research could be done, I think I have learned enough to be confident there is no silver bullet. I am happy to keep polishing a long-term solution but I'm sure @crandmck is eager the problem to be fixed & for me to move on to other work. Here's how I plan to move forward for now:
This will solve our immediate problem as well as give us a more comprehensive view of how significant this issue is. * it's referring to multiple paragraphs in lists but it effectively refers to any type of block level element in a list: "List items may consist of multiple paragraphs. Each subsequent paragraph in a list item must be indented by either 4 spaces or one tab:" |
|
Also, I'm in the office today so please let me know if you'd like to call-- I think the thread's gotten long enough that real-time hashing-out is advisable at this point. 😸 |
Sequoia
added a commit
to Sequoia/loopback-example-kv-connectors
that referenced
this issue
Oct 12, 2016
Sequoia
added a commit
to Sequoia/loopback-example-middleware
that referenced
this issue
Oct 12, 2016
Sequoia
added a commit
to Sequoia/loopback-example-mixins
that referenced
this issue
Oct 12, 2016
This was referenced Oct 12, 2016
Sequoia
added a commit
to Sequoia/loopback-example-storage
that referenced
this issue
Oct 12, 2016
Sequoia
added a commit
to Sequoia/loopback-example-user-management
that referenced
this issue
Oct 12, 2016
|
I've reviewed all markdown files in |
superkhau
pushed a commit
to strongloop/loopback-example-access-control
that referenced
this issue
Oct 12, 2016
superkhau
pushed a commit
to strongloop-community/loopback-android-getting-started
that referenced
this issue
Oct 12, 2016
superkhau
pushed a commit
to strongloop-community/loopback-example-connector
that referenced
this issue
Oct 12, 2016
superkhau
pushed a commit
to strongloop/loopback-example-database
that referenced
this issue
Oct 12, 2016
superkhau
pushed a commit
to strongloop/loopback-example-kv-connectors
that referenced
this issue
Oct 12, 2016
superkhau
pushed a commit
to strongloop-community/loopback-example-middleware
that referenced
this issue
Oct 12, 2016
superkhau
pushed a commit
to strongloop/loopback-example-mixins
that referenced
this issue
Oct 12, 2016
superkhau
pushed a commit
to strongloop/loopback-example-user-management
that referenced
this issue
Oct 12, 2016
superkhau
pushed a commit
to strongloop/loopback-example-storage
that referenced
this issue
Oct 12, 2016
|
Discussion among @Sequoia @superkhau and @crandmck came to following conclusions:
@bajtos FYI |
crandmck
pushed a commit
that referenced
this issue
Apr 25, 2017
* Fix typo * Fix a 404 * Point [hooks] to operation-hooks * Link to issue #49 * Remove the video on user management Its bad audio quality casts a negative light on LoopBack * Delist POST as a method for an Update
wanedenis
pushed a commit
to wanedenis/GettingStartedWithLoopbackJS
that referenced
this issue
Jan 10, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Come up with a proposal to answer the following:
The text was updated successfully, but these errors were encountered: