Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fontify Grimoire code blocks if possible #2007

Merged
merged 1 commit into from
Jun 13, 2017
Merged

Fontify Grimoire code blocks if possible #2007

merged 1 commit into from
Jun 13, 2017

Conversation

xiongtx
Copy link
Member

@xiongtx xiongtx commented Jun 10, 2017

Follow-up on #1959 and the discussion with @bbatsov r.e. fontifying code blocks in the markdown buffer returned by cider-grimoire.

Thanks to @jrblevin for addressing jrblevin/markdown-mode#185.

@xiongtx xiongtx force-pushed the cider-grimoire-markdown branch 2 times, most recently from 67a9bd1 to c1ec450 Compare June 10, 2017 03:36
@bbatsov
Copy link
Member

bbatsov commented Jun 10, 2017

Looks good! Just add a changelog entry, refer in the commit message to the ticket this is solving and take care of the byte-compilation warnings.

@xiongtx
Copy link
Member Author

xiongtx commented Jun 10, 2017 •
edited
Loading

I'm not 100% sure if with-no-warnings is the right way to address byte-compiler warnings in this situation. I've posted a question on the Emacs StackExchange regarding this.

I'm also not sure why build 4636.5 failed while others pass. Is it just a stricter setting that treats certain warnings as errors?

Update: seems like the recommended way (using declare) doesn't pass the test that actually checks for the existence of markdown-mode in markdown-mode.el, since markdown-mode.el is not available. This is nonetheless safe to merge.

@xiongtx xiongtx force-pushed the cider-grimoire-markdown branch 3 times, most recently from f779e28 to 82b85b1 Compare June 11, 2017 04:55
bbatsov
bbatsov reviewed Jun 13, 2017
View reviewed changes
cider-grimoire.el
@@ -35,6 +35,8 @@

(require 'url-vars)

(declare-function markdown-mode "markdown-mode.el")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You also have to declare markdown-toggle-fontify-code-blocks-natively.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One more thing - for this work markdown-mode should actually be present. You can just add markdown-mode to the dev dependencies in Cask.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@bbatsov bbatsov merged commit 238c60c into clojure-emacs:master Jun 13, 2017
@bbatsov
Copy link
Member

bbatsov commented Jun 13, 2017

👍

@xiongtx xiongtx deleted the cider-grimoire-markdown branch June 13, 2017 19:55
@xiongtx xiongtx mentioned this pull request Jul 16, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bbatsov bbatsov bbatsov left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@xiongtx @bbatsov