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

Add support for rendering atx headers in the margin #274

Closed
wants to merge 9 commits into from

Conversation

algal
Copy link
Contributor

@algal algal commented Oct 17, 2017

This commit introduces three new customization variables, which affect
how the buffer is rendered by controlling the presentation of marginalized
headers
.

  • markdown-marginalize-header

    When it's on, any opening atx headers will be rendered in a left margin of
    the window. Off by default. Because this setting relies on the window's
    left margin, it will conflict with other minor modes that want to own the
    left margin, like linum-mode.

  • markdown-marginalize-header-in-header-face

    Whether the marginalized header marks should themselves be displayed
    in the face used for header text. Off by default.

    If you turn it on, you will probably also need to adjust the following
    variable, markdown-marginalize-header-margin-size.

  • markdown-marginalize-header-margin-size

    Size of the left margin, by default, seven. You will probably need to increase
    this value if you want your marginalized headers to be rendered
    in header faces, since seven text-sized characters will not be wide enough
    to accommodate the rendering of seven header-sized octothorps.

algal added 5 commits October 14, 2017 23:25
These variables are now working:

- markdown-marginalize-header

  Turns this header marginalization on/off. When it's on, the mode
  will configure the window appropriately. Off by default.

- markdown-marginalize-header-in-header-face

  Whether the marginalized header marks should themselves be displayed
  in the header face. Off by default.

  If you turn it on, you will probably also need to adjust your margin
  size manually using markdown-marginalize-header-margin-size, or else
  the header-faced markup characters will not fit in the window margin
  that was sized based on text-faced characters.

- markdown-marginalize-header-margin-size

  Size of the left margin, by default, seven. You will need to adjust
  this manually if you want your marginalized headers to be rendered
  in header faces.
@jrblevin
Copy link
Owner

Thanks for putting this together. I had a few thoughts on how to simplify things and I have been working on those. I'm out of time for now, but I pushed a commit to my feat/marginal-headers branch. Here are the changes (and sorry for the messy bulk commit):

  • Make header marginalization compatible with markup hiding. Don't hide trailing markup, if any. That can be hidden already with markup hiding.

  • Remove markdown-marginalize-header-in-header-face. Always use markdown-header-delimiter-face instead. Users can customize that face already if they wish for the markup to appear in the heading face.

  • Remove markdown-marginalize-header-margin-size. Calculate the required margin size on the fly based on character width of markdown-header-delimiter-face.

  • generate-margin-string pollutes the global namespace; rename to markdown--marginalize-string instead.

  • Rename defcustom to markdown-marginalize-headers (plural–as I probably should have done with markdown-asymmetric-header) and declare safe as boolean.

Check it out when you have time and let me know what you think. Thanks again.

@jrblevin
Copy link
Owner

Unfortunately I just noticed that it's incompatible with Emacs 24 due to the font width functions I used.

@jrblevin
Copy link
Owner

I took the liberty of squashing and rebasing your commits into one (committed as 39a0c57). Then I applied my changes in a separate commit (0167557). The net effect is probably easier to see in the combined patch. I'm not sure how to link to that on GitHub, but you can see it with git diff 8842181..0167557. Let me know if you have comments or concerns and thanks again for the patch.

@algal
Copy link
Contributor Author

algal commented Oct 19, 2017

Cool! I’ll have a look tonight.

@algal
Copy link
Contributor Author

algal commented Oct 24, 2017

By the way, I have been using this successfully for the last few days. It looks good to me!

Since it seems like you have already squashed and merged this work onto master, I suppose the next step is just to close this pull request? I'll leave that to you, since I'm not sure if you have another workflow in mind, which you prefer.

@algal
Copy link
Contributor Author

algal commented Oct 24, 2017

I should add that one slight issue I have seen very intermittently is that sometimes the mode does not seem to apply the set-window-margins reliably. This is very infrequent, so I have not been able to reproduce it. But I'm keeping an eye open for it in case there is some other hook where we should make make that call, rather than just in markdown-marginalize-update-current.

@jrblevin
Copy link
Owner

I was just leaving this open as a reminder in case there were any problems. Thanks for confirming, once again for the patch. I really like this and I'm leaving it on full-time!

I've also noticed the delay in setting the margins. It's not a huge deal, but indeed we should look into it.

@jrblevin jrblevin closed this Oct 24, 2017
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.

2 participants