search

alphapapa / org-bookmark-heading

Emacs bookmark support for Org-mode
GNU General Public License v3.0
93 stars 8 forks source link

Use default bookmark generator if we're before the first heading #4

Closed mm-- closed 7 years ago

mm-- commented 7 years ago

Great package.

One minor issue is setting bookmarks when we're before the first heading. In this case, org-heading-components errors out.

This change goes back to using bookmark-make-record-default in the case that we're before the first heading (e.g. if we wanted to bookmark the file instead of a particular heading).

alphapapa commented 7 years ago

Hey there,

Good catch! I can't tell you how many times before-first-heading situations have caught me in Org stuff. Almost makes me want to define a with-first-heading macro, haha.

Couple of questions:

  1. Is the (require 'bookmark) necessary? I'm guessing it's loaded by default in Emacs, although I wouldn't be surprised if this silences a compiler warning, in which case it should definitely be used.
  2. Looking at the signature of bookmark-make-record-default, what do you think about the NO-CONTEXT argument? Do you think it'd be better to set this to t? I'm thinking about, e.g. header lines (#+TITLE:, etc). If those change, it would break the bookmark record, right? But if the bookmark just records the position, it should keep working.

Thanks a lot for submitting this.

By the way, I looked at your blog, and your March 7th post about refiling with Hydras is awesome! Is your blog on Planet Emacs? I don't remember seeing that post up there. If not, you should definitely add it!

mm-- commented 7 years ago

I just added the (require 'bookmark) because of a couple flycheck warnings about

  1. assignment to free variable ‘bookmark-make-record-function’ on line 18
  2. the following functions are not known to be defined: bookmark-make-record-default on line 85

I don't know if requiring the bookmark package would slow anything down.

Ah, ok. It won't slow anything down, so I guess we can leave it in to silence the warnings.

As for the NO-CONTEXT argument, it's ultimately your call, but I'm finding that including context makes the bookmarks a bit more resilient to change. The character position of the bookmark is still stored even with context, and bookmark-default-handler seems to fall back on the position if it can't find either the front-context or the rear-context.

Ok, that sounds great.

Thanks for the encouraging suggestion about adding my blog to Planet Emacs! Once I get my static site generator to generate RSS feeds for the Emacs category, I'll definitely add it. :smile:

:D

Thanks, really appreciate your contribution.