feat: Support parsing of SQL code blocks in Markdown files#598
feat: Support parsing of SQL code blocks in Markdown files#598michael-the1 wants to merge 18 commits intotconbeer:mainfrom
Conversation
|
Thank you for this! I'm mostly away from my computer this week but I'll try to review next week. |
tconbeer
left a comment
There was a problem hiding this comment.
Wow, thank you!
Just a couple of nits, honestly. Really well done with this. I really, really appreciate the tests in particular. GHA is down right now, so the checks won't run, but assuming those pass I think we can get this merged really soon.
src/sqlfmt/api.py
Outdated
There was a problem hiding this comment.
Let's wrap this in try ... except ImportError and either return the source or re-raise a SqlfmtError to create a nicer error message for users who don't have the extra installed. I know right now we shouldn't hit that codepath due to the Mode property, but would be good to safeguard against that changing in the future.
There was a problem hiding this comment.
I added the try ... except guard. I went ahead and subclassed SqlfmtError since I didn't see any bare raise SqlfmtError anywhere.
|
@michael-the1 Sorry I dropped the ball on this. I'd love to get this finalized. Looks like there are quite a few failing tests and lints. Do you want to work through those? Let me know if you get stuck. |
|
@tconbeer Life can get busy which I totally understand. The reason the tests are failing is because |
|
Ah, I see now that, in the Github workflow, dependencies are installed with I also see some other errors in the static analysis and the primer workflows. I'll take a look at those tomorrow. |
3e3c1df to
35ace60
Compare
|
@tconbeer I think I fixed all issues, but with two notes:
|
|
@tconbeer Would you reconsider this feature? It would be very useful to keep markdown documentation consistent, when they include many example SQL queries - which is definitely my case 🙂 |
Closes #593
There are two caveats that would be good to consider:
mistletoekeeps all the important whitespace, but can toss out some unnecessary whitespace, such as trailing whitespace or double newlines. A good example is in theREADME.mdof this very repo:I think this is acceptable since it does not affect readability and should not affect output (i.e. when it's rendered to HTML).
sqlblocks are not formatted. I think this is acceptable. See example: