[PATCH] Make the point visible when jumping to the mark

[PATCH] Make the point visible when jumping to the mark

[Ian Kelling]

From 9191e4a364e251119cf8b7c72e41f6c0d09583f2 Mon Sep 17 00:00:00 2001
Message-ID: <87ha5aqa93.fsf@treetowl.lan>
MIME-Version: 1.0
Content-Type: text/plain

*lisp/org.el: Advise commands which jump to the mark
---

There are several non-org commands that jump to a location and would be
unwieldy if the location remained hidden, (isearch, bookmark-jump,
save-place), but org-mode has code to fix them. In this patch, I
followed their example.

I have an emacs fsf copyright assignment completed & on file with fsf, I can
send gpg signed copy if you need it.

- Ian Kelling


 lisp/org.el |   21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/lisp/org.el b/lisp/org.el
index 44a4e44..9365059 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -24326,6 +24326,27 @@ To get rid of the restriction, use \\[org-agenda-remove-restriction-lock]."
 			   (outline-invisible-p)))
        (org-show-context 'bookmark-jump)))
 
+(eval-after-load "simple"
+  '(defadvice set-mark-command (after org-make-visible activate)
+     "Make the point visible with `org-show-context'."
+     (org-mark-jump-unhide)))
+
+(eval-after-load "simple"
+  '(defadvice exchange-point-and-mark (after org-make-visible activate)
+     "Make the point visible with `org-show-context'."
+     (org-mark-jump-unhide)))
+
+(eval-after-load "simple"
+  '(defadvice pop-global-mark (after org-make-visible activate)
+     "Make the point visible with `org-show-context'."
+     (org-mark-jump-unhide)))
+
+(defun org-mark-jump-unhide ()
+  "Make the point visible with `org-show-context' after jumping to the mark."
+  (when (and (derived-mode-p 'org-mode)
+	     (outline-invisible-p))
+    (org-show-context 'mark-goto)))
+
 ;; Make session.el ignore our circular variable
 (defvar session-globals-exclude)
 (eval-after-load "session"
-- 
1.7.10.4

Re: [PATCH] Make the point visible when jumping to the mark

[Ian Kelling]

> From 9191e4a364e251119cf8b7c72e41f6c0d09583f2 Mon Sep 17 00:00:00 2001
> Message-ID: <87ha5aqa93.fsf@treetowl.lan>
> MIME-Version: 1.0
> Content-Type: text/plain
>
> *lisp/org.el: Advise commands which jump to the mark
> ---
>
> There are several non-org commands that jump to a location and would be
> unwieldy if the location remained hidden, (isearch, bookmark-jump,
> save-place), but org-mode has code to fix them. In this patch, I
> followed their example.
>
> I have an emacs fsf copyright assignment completed & on file with fsf, I can
> send gpg signed copy if you need it.
>
> - Ian Kelling
>
>
>  lisp/org.el |   21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
>
> diff --git a/lisp/org.el b/lisp/org.el
> index 44a4e44..9365059 100644
> --- a/lisp/org.el
> +++ b/lisp/org.el
> @@ -24326,6 +24326,27 @@ To get rid of the restriction, use \\[org-agenda-remove-restriction-lock]."
>  			   (outline-invisible-p)))
>         (org-show-context 'bookmark-jump)))
>  
> +(eval-after-load "simple"
> +  '(defadvice set-mark-command (after org-make-visible activate)
> +     "Make the point visible with `org-show-context'."
> +     (org-mark-jump-unhide)))
> +
> +(eval-after-load "simple"
> +  '(defadvice exchange-point-and-mark (after org-make-visible activate)
> +     "Make the point visible with `org-show-context'."
> +     (org-mark-jump-unhide)))
> +
> +(eval-after-load "simple"
> +  '(defadvice pop-global-mark (after org-make-visible activate)
> +     "Make the point visible with `org-show-context'."
> +     (org-mark-jump-unhide)))
> +
> +(defun org-mark-jump-unhide ()
> +  "Make the point visible with `org-show-context' after jumping to the mark."
> +  (when (and (derived-mode-p 'org-mode)
> +	     (outline-invisible-p))
> +    (org-show-context 'mark-goto)))
> +
>  ;; Make session.el ignore our circular variable
>  (defvar session-globals-exclude)
>  (eval-after-load "session"

I posted this patch in September. It seems it was forgotten. I posted it
again on April 30th, nothing yet. I also have another patch thats been
sitting on the list for a few weeks now after having a discussion and a
positive response. I'd like to do a git pull and find my patches applied
at some point, and I'm wondering if/when that will happen. And is there
anything I can do to help?

- Ian Kelling

Re: [PATCH] Make the point visible when jumping to the mark

[Bastien]

Hi Ian,

Ian Kelling <ian@iankelling.org> writes:

> I posted this patch in September. It seems it was forgotten.

I missed it as the time.

> I posted it again on April 30th, nothing yet.  I also have another
> patch thats been sitting on the list for a few weeks now after
> having a discussion and a positive response.

Yes, in http://article.gmane.org/gmane.emacs.orgmode/86050

Did you receive the FSF confirmation for your copyright assignment?
That's the information we need before applying the patches.

> I'd like to do a git pull and find my patches applied at some point,
> and I'm wondering if/when that will happen. And is there anything I
> can do to help?

If the process is completed let us know and we will move on.

Thanks!

-- 
 Bastien

Re: [PATCH] Make the point visible when jumping to the mark

[Ian Kelling]

Bastien <bzg@gnu.org> writes:

> Hi Ian,
>
> Ian Kelling <ian@iankelling.org> writes:
>
>> I posted this patch in September. It seems it was forgotten.
>
> I missed it as the time.
>
>> I posted it again on April 30th, nothing yet.  I also have another
>> patch thats been sitting on the list for a few weeks now after
>> having a discussion and a positive response.
>
> Yes, in http://article.gmane.org/gmane.emacs.orgmode/86050
>
> Did you receive the FSF confirmation for your copyright assignment?
> That's the information we need before applying the patches.

Yes, I've got the confirmation. All set.

>> I'd like to do a git pull and find my patches applied at some point,
>> and I'm wondering if/when that will happen. And is there anything I
>> can do to help?
>
> If the process is completed let us know and we will move on.
>
> Thanks!

Great to hear. Yes, all completed.

Thanks,
Ian

Re: [PATCH] Make the point visible when jumping to the mark

[Bastien]

Hi Ian,

Please send me your public key so that I can give you access to the
org-mode and worg repositories.

Ian Kelling <ian@iankelling.org> writes:

> From 9191e4a364e251119cf8b7c72e41f6c0d09583f2 Mon Sep 17 00:00:00 2001
> Message-ID: <87ha5aqa93.fsf@treetowl.lan>
> MIME-Version: 1.0
> Content-Type: text/plain
>
> *lisp/org.el: Advise commands which jump to the mark

The formatting of the Changelog message not good.

It should be

* org.el: Advise commands which jump to the mark.

See http://orgmode.org/worg/org-contribute.html and example in
the git logs.

> +(eval-after-load "simple"
> +  '(defadvice set-mark-command (after org-make-visible activate)
> +     "Make the point visible with `org-show-context'."
> +     (org-mark-jump-unhide)))

Why this one?  The user don't want to use C-SPC in hidden parts.

> +(eval-after-load "simple"
> +  '(defadvice exchange-point-and-mark (after org-make-visible activate)
> +     "Make the point visible with `org-show-context'."
> +     (org-mark-jump-unhide)))

> +(eval-after-load "simple"
> +  '(defadvice pop-global-mark (after org-make-visible activate)
> +     "Make the point visible with `org-show-context'."
> +     (org-mark-jump-unhide)))

This two ones are good, yes.

Thanks,

-- 
 Bastien

Re: [PATCH] Make the point visible when jumping to the mark

[Ian Kelling]

[-- Attachment #1: Type: text/plain, Size: 686 bytes --]

Bastien <bzg@gnu.org> writes:
> The formatting of the Changelog message not good.
>
> It should be
>
> * org.el: Advise commands which jump to the mark.
>
> See http://orgmode.org/worg/org-contribute.html and example in
> the git logs.
>

My mistake. Fixed in the attached patch.

>> +(eval-after-load "simple"
>> +  '(defadvice set-mark-command (after org-make-visible activate)
>> +     "Make the point visible with `org-show-context'."
>> +     (org-mark-jump-unhide)))
>
> Why this one?  The user don't want to use C-SPC in hidden parts.

Yes, I agree. They want to use C-u C-SPC to go to hidden parts. I've
updated the patch to advise pop-to-mark-command instead. It is attached.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Make-the-point-visible-when-jumping-to-the-mark.patch --]
[-- Type: text/x-diff, Size: 1528 bytes --]

From ead59ed465eb33e822dd69f6e34b087477ebc239 Mon Sep 17 00:00:00 2001
From: Ian Kelling <ian@iankelling.org>
Date: Wed, 30 Apr 2014 21:54:08 -0700
Subject: [PATCH] Make the point visible when jumping to the mark

* org.el: Advise commands which jump to the mark
---
 lisp/org.el |   21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/lisp/org.el b/lisp/org.el
index 40e16ea..17f7997 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -24510,6 +24510,27 @@ To get rid of the restriction, use \\[org-agenda-remove-restriction-lock]."
 			   (outline-invisible-p)))
        (org-show-context 'bookmark-jump)))
 
+(eval-after-load "simple"
+  '(defadvice pop-to-mark-command (after org-make-visible activate)
+     "Make the point visible with `org-show-context'."
+     (org-mark-jump-unhide)))
+
+(eval-after-load "simple"
+  '(defadvice exchange-point-and-mark (after org-make-visible activate)
+     "Make the point visible with `org-show-context'."
+     (org-mark-jump-unhide)))
+
+(eval-after-load "simple"
+  '(defadvice pop-global-mark (after org-make-visible activate)
+     "Make the point visible with `org-show-context'."
+     (org-mark-jump-unhide)))
+
+(defun org-mark-jump-unhide ()
+  "Make the point visible with `org-show-context' after jumping to the mark."
+  (when (and (derived-mode-p 'org-mode)
+	     (outline-invisible-p))
+    (org-show-context 'mark-goto)))
+
 ;; Make session.el ignore our circular variable
 (defvar session-globals-exclude)
 (eval-after-load "session"
-- 
1.7.10.4

Re: [PATCH] Make the point visible when jumping to the mark

[Bastien]

Hi Ian,

Ian Kelling <ian@iankelling.org> writes:

> Yes, I agree. They want to use C-u C-SPC to go to hidden parts. I've
> updated the patch to advise pop-to-mark-command instead. It is attached.

Thanks, applied, I added a fullstop at the end of the Changelog commit.

As explain in my reply to Sébastien, I will move this code into a
separate library.

-- 
 Bastien

Re: [PATCH] Make the point visible when jumping to the mark

[Sebastien Vauban]

Bastien wrote:
> Hi Ian,
>
> Please send me your public key so that I can give you access to the
> org-mode and worg repositories.
>
> Ian Kelling <ian-CsVmbpwo38eXwddmVfQv5g@public.gmane.org> writes:
>
>> From 9191e4a364e251119cf8b7c72e41f6c0d09583f2 Mon Sep 17 00:00:00 2001
>> Message-ID: <87ha5aqa93.fsf-OUiqzQ9FT5r+AlalS6MPSg@public.gmane.org>
>> MIME-Version: 1.0
>> Content-Type: text/plain
>>
>> *lisp/org.el: Advise commands which jump to the mark
>
> The formatting of the Changelog message not good.
>
> It should be
>
> * org.el: Advise commands which jump to the mark.
>
> See http://orgmode.org/worg/org-contribute.html and example in
> the git logs.
>
>> +(eval-after-load "simple"
>> +  '(defadvice set-mark-command (after org-make-visible activate)
>> +     "Make the point visible with `org-show-context'."
>> +     (org-mark-jump-unhide)))
>>
>> +(eval-after-load "simple"
>> +  '(defadvice exchange-point-and-mark (after org-make-visible activate)
>> +     "Make the point visible with `org-show-context'."
>> +     (org-mark-jump-unhide)))
>
>> +(eval-after-load "simple"
>> +  '(defadvice pop-global-mark (after org-make-visible activate)
>> +     "Make the point visible with `org-show-context'."
>> +     (org-mark-jump-unhide)))
>
> This two ones are good, yes.

Even if the goal is desirable, I thought that we may not add defadvice
in Emacs sources.

(... a couple of minutes later ...)

Well, Googling returns this :

  ╭──── http://www.gnu.org/software/emacs/manual/html_node/elisp/Advising-Functions.html
  │
  │ Emacs's own source files should not put advice on functions in
  │ Emacs. There are currently a few exceptions to this convention, but we
  │ aim to correct them.
  ╰────

So, it seems this should be avoided as long as Org does go into Emacs.

Best regards,
  Seb

-- 
Sebastien Vauban

Re: [PATCH] Make the point visible when jumping to the mark

[Bastien]

Hi Sébastien,

Sebastien Vauban <sva-news-D0wtAvR13HarG/iDocfnWg@public.gmane.org>
writes:

> Even if the goal is desirable, I thought that we may not add defadvice
> in Emacs sources.

True that, we need to clean things up.

The route I will take is to apply Ian patch on master and then to
move all advising code into a separate org-advice.el library in the
contrib/ directory.  Any suggestion for a better name?

-- 
 Bastien

Re: [PATCH] Make the point visible when jumping to the mark

[Ian Kelling]

Bastien <bzg@gnu.org> writes:

> Hi Sébastien,
>
> Sebastien Vauban <sva-news-D0wtAvR13HarG/iDocfnWg@public.gmane.org>
> writes:
>
>> Even if the goal is desirable, I thought that we may not add defadvice
>> in Emacs sources.
>
> True that, we need to clean things up.
>
> The route I will take is to apply Ian patch on master and then to
> move all advising code into a separate org-advice.el library in the
> contrib/ directory.  Any suggestion for a better name?

I considered this problem when I made the patch, and originally started
writing it to not use advise. The alternative I see is to create
org-mode versions of these functions, and binding them with the
org-mode-map to override the existing functions, like we do with other
functions. I prefer advice in these cases because the advice is simple
and it is easier for users customizing keybinds and extending
emacs. However, compared to making it a module and thus not enabled by
default, I would prefer to make org-mode versions of the
functions. Having them on by default will be more helpful to more users
overall. I would be happy to write the patch for this, if we agree.

Re: [PATCH] Make the point visible when jumping to the mark

[Bastien]

Hi Ian,

Ian Kelling <ian@iankelling.org> writes:

> I would be happy to write the patch for this, if we agree.

Anything that gets rid of advice is good, we agree on this.

If you have time, please start sketching out the patch so that we can
discuss how to replace some of the advice by internal Org functions.
Note that we will certainly hit some roadblocks, and we don't want to
replace core Emacs fonctions like `pop-to-mark'.

One possible outcome is to consider a new Emacs hook: something like
`after-interactive-point-move-hook' that would check if the current
buffer is in Org and would reveal the context systematically.

I haven't thought this through, but raising this needs on the
emacs-devel list will perhaps leads to something.

Otherwise, having a contributed package with advice (using
add-function instead of defadvice) would be good enough.

Thanks!

-- 
 Bastien

[PATCH] Make the point visible when jumping to the mark

[Ian Kelling]

[-- Attachment #1: Type: text/plain, Size: 859 bytes --]

An example of the problem:

* headline-1
** one of many headlines I want to remain folded...
** headline
*** headline
The point starts here. I set a mark on this line.
Then I fold headline-1. This becomes invisible.
* headline-2
I move the point here. After I'm done here, I want to go back to my
mark. I do C-u C-space (pop-to-mark-command), but it doesn't go to the
mark. Instead, it takes me to the end of the still folded
headline-1. To get back to my mark without globally unfolding
everything requires tediously unfolding each headline and sub-headline
until the marked location is exposed.

There are some other non-org commands that would have this same
problem (isearch, bookmark-jump, save-place), but org-mode has code to
fix them. org-mark-ring-goto also handles this. I followed those examples
and wrote a patch. It is attached.

- Ian Kelling



[-- Attachment #2: 0001-Make-the-point-visible-when-jumping-to-the-mark.patch --]
[-- Type: text/x-patch, Size: 1612 bytes --]

From 066595dd9350f06a1df2e99a341f96782ac8dfed Mon Sep 17 00:00:00 2001
From: Ian Kelling <ianowl@gmail.com>
Date: Sun, 15 Sep 2013 00:32:08 -0700
Subject: [PATCH] Make the point visible when jumping to the mark

* lisp/org.el advise `pop-global-mark', `exchange-point-and-mark', and
  `pop-global-mark' with `org-show-context', as appropriate.

TINYCHANGE
---
 lisp/org.el | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/lisp/org.el b/lisp/org.el
index 8e970a1..30b87d9 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -23903,6 +23903,27 @@ To get rid of the restriction, use \\[org-agenda-remove-restriction-lock]."
 			   (outline-invisible-p)))
        (org-show-context 'bookmark-jump)))
 
+(eval-after-load "simple"
+  '(defadvice set-mark-command (after org-make-visible activate)
+     "Make the point visible with `org-show-context'."
+     (org-mark-jump-unhide)))
+
+(eval-after-load "simple"
+  '(defadvice exchange-point-and-mark (after org-make-visible activate)
+     "Make the point visible with `org-show-context'."
+     (org-mark-jump-unhide)))
+
+(eval-after-load "simple"
+  '(defadvice pop-global-mark (after org-make-visible activate)
+     "Make the point visible with `org-show-context'."
+     (org-mark-jump-unhide)))
+
+(defun org-mark-jump-unhide ()
+  "Make the point visible with `org-show-context' after jumping to the mark."
+  (and (derived-mode-p 'org-mode)
+       (outline-invisible-p)
+       (org-show-context 'mark-goto)))
+
 ;; Make session.el ignore our circular variable
 (defvar session-globals-exclude)
 (eval-after-load "session"
-- 
1.8.3.1

Re: [PATCH] Make the point visible when jumping to the mark

[Ian Kelling]

[-- Attachment #1: Type: text/plain, Size: 47 bytes --]

Updated patch. Tiny commit message format fix.

[-- Attachment #2: 0001-Make-the-point-visible-when-jumping-to-the-mark.patch --]
[-- Type: text/x-patch, Size: 1613 bytes --]

From c46db11ffe0cc0f92222b4fbd5a1ede2b4be0175 Mon Sep 17 00:00:00 2001
From: Ian Kelling <ianowl@gmail.com>
Date: Sun, 15 Sep 2013 00:32:08 -0700
Subject: [PATCH] Make the point visible when jumping to the mark

* lisp/org.el: advise `pop-global-mark', `exchange-point-and-mark', and
  `pop-global-mark' with `org-show-context', as appropriate.

TINYCHANGE
---
 lisp/org.el | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/lisp/org.el b/lisp/org.el
index 8e970a1..30b87d9 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -23903,6 +23903,27 @@ To get rid of the restriction, use \\[org-agenda-remove-restriction-lock]."
 			   (outline-invisible-p)))
        (org-show-context 'bookmark-jump)))
 
+(eval-after-load "simple"
+  '(defadvice set-mark-command (after org-make-visible activate)
+     "Make the point visible with `org-show-context'."
+     (org-mark-jump-unhide)))
+
+(eval-after-load "simple"
+  '(defadvice exchange-point-and-mark (after org-make-visible activate)
+     "Make the point visible with `org-show-context'."
+     (org-mark-jump-unhide)))
+
+(eval-after-load "simple"
+  '(defadvice pop-global-mark (after org-make-visible activate)
+     "Make the point visible with `org-show-context'."
+     (org-mark-jump-unhide)))
+
+(defun org-mark-jump-unhide ()
+  "Make the point visible with `org-show-context' after jumping to the mark."
+  (and (derived-mode-p 'org-mode)
+       (outline-invisible-p)
+       (org-show-context 'mark-goto)))
+
 ;; Make session.el ignore our circular variable
 (defvar session-globals-exclude)
 (eval-after-load "session"
-- 
1.8.3.1