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.

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

Respect font-lock-mode nil. #3

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

tommay
Copy link
Contributor

@tommay tommay commented Mar 2, 2015

font-lock-refresh-defaults can set font-lock-mode to t even if global-font-lock-mode and font-lock-global-modes are nil. So save the original value of font-lock-mode and set it back to nil if it was nil to begin with.

This is probably an issue with font-lock-refresh-defaults but this hack makes markdown-mode readable for me.

font-lock-refresh-defaults can set font-lock-mode to t even
if global-font-lock-mode and font-lock-global-modes are nil.
So save the original value of font-lock-mode and set it back
to nil if it was nil to begin with.
@kini
Copy link
Contributor

kini commented Sep 2, 2015

You may wish to submit this to http://github.com/jrblevin/markdown-mode instead, since that's the official repo. This one is just a mirror.

@tommay
Copy link
Contributor Author

tommay commented Sep 2, 2015

Ooops, thanks!

On Wed, Sep 2, 2015 at 3:04 PM, Keshav Kini [email protected]
wrote:

You may wish to submit this to jrblevin/markdown-mode instead, since
that's the official repo. This one is just a mirror.


Reply to this email directly or view it on GitHub
#3 (comment).

@jrblevin
Copy link
Contributor

I just merged this in to the main repository. Thanks for the patch and sorry for the long delay in getting it in. I haven't been checking this mirror for pull requests.

For future reference, markdown-mode is now on GitHub officially here: https://github.com/jrblevin/markdown-mode

@ncihnegn
Copy link

This just ruined my highlighting.

@jrblevin
Copy link
Contributor

Do you have font-lock-mode turned off globally? If I understand, the original problem was that when font-lock-mode was disabled globally, it would get turned back on by markdown-mode when markdown-reload-extensions ran. Seems that @tommay understandably wanted markdown-mode to respect the settings and leave it off.

I checked and the entire body of font-lock-refresh-defaults is simply this:

(font-lock-mode -1)
(kill-local-variable 'font-lock-set-defaults)
(font-lock-mode 1)

So, I'm now not convinced that this patch is the best way to solve the problem. I will look into other solutions.

@jrblevin
Copy link
Contributor

I confirmed the issue with --no-init-file, so I reverted the commit while I look for another approach.

@tommay
Copy link
Contributor Author

tommay commented Jan 11, 2016

There's probably some reason why I didn't just do

(when (and (font-lock-mode) (fboundp 'font-lock-refresh-defaults))
(font-lock-refresh-defaults))

but the font-lock code is so twisted I don't remember what it was. Maybe I
thought it would be self-evident to those who deal with font-lock more
regularly. I just like to turn off font-lock mode and be done with it,
since my vision is somewhat poor.

On Mon, Jan 11, 2016 at 5:28 AM, Jason Blevins [email protected]
wrote:

I confirmed the issue with --no-init-file, so I reverted the commit while
I look for another approach.


Reply to this email directly or view it on GitHub
#3 (comment).

@tommay
Copy link
Contributor Author

tommay commented Jan 13, 2016

I remembered why I did this patch by saving and restoring font-lock-mode: it's not apparent why font-lock-refresh-defaults is being called here, or what it's doing that is useful. But, it was enabling font-lock-mode which should be a decision made by the user (markdown-mode should/does work fine without it AFAIK). So to keep whatever functionality it may be doing that is useful or required, but keep it from overstepping its bounds and enabling font-lock-mode, I did the save/restore.

But it looks like that doesn't work. Oops.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants