[PATCH] org-id-goto doesn't work if buffer is narrowed.

[PATCH] org-id-goto doesn't work if buffer is narrowed.

[Puneeth Chaganti]

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

Hello,

I've attached a one-line patch to fix `org-id-goto' if the target
buffer is narrowed. Let me know if this fix sounds reasonable or if
you have other suggestions!

Thanks,
Puneeth

[-- Attachment #2: 0001-Widen-buffer-in-org-id-goto.patch --]
[-- Type: text/x-patch, Size: 756 bytes --]

From 3f3deb84fc2e2739307e820cdfa7de1c6fbf4f23 Mon Sep 17 00:00:00 2001
From: Puneeth Chaganti <punchagan@muse-amuse.in>
Date: Fri, 23 Oct 2015 20:21:59 +0530
Subject: [PATCH] Widen buffer in `org-id-goto'.

When the target buffer for `org-id-goto' is already open and narrowed,
it doesn't work.
---
 lisp/org-id.el | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lisp/org-id.el b/lisp/org-id.el
index f86ef22..149a69b 100644
--- a/lisp/org-id.el
+++ b/lisp/org-id.el
@@ -296,6 +296,7 @@ Move the cursor to that entry in that buffer."
     (unless m
       (error "Cannot find entry with ID \"%s\"" id))
     (org-pop-to-buffer-same-window (marker-buffer m))
+    (widen)
     (goto-char m)
     (move-marker m nil)
     (org-show-context)))
-- 
2.1.4

Re: [PATCH] org-id-goto doesn't work if buffer is narrowed.

[Rasmus]

Hi Puneeth,

Thanks for your patch.

Puneeth Chaganti <punchagan@gmail.com> writes:

> I've attached a one-line patch to fix `org-id-goto' if the target
> buffer is narrowed. Let me know if this fix sounds reasonable or if
> you have other suggestions!

It's not obvious that org should change a—potentially—carefully selected
narrowed region.

Perhaps you could mimic the way org-edit-special works for this case.

Thanks,
Rasmus

-- 
And when I’m finished thinking, I have to die a lot

Re: [PATCH] org-id-goto doesn't work if buffer is narrowed.

[Puneeth Chaganti]

Hi Rasmus,

On Fri, Oct 23, 2015 at 8:57 PM, Rasmus <rasmus@gmx.us> wrote:
> It's not obvious that org should change a—potentially—carefully selected
> narrowed region.

I agree. But, am I not explicitly asking to jump to the specified
item. I don't mind the widening, at least when the call is
interactive. I agree with you when some other code is calling it,
though.

> Perhaps you could mimic the way org-edit-special works for this case.

You mean, display the entry in a new buffer, and any changes will be
applied onto the original entry too?

-- 
Puneeth

Re: [PATCH] org-id-goto doesn't work if buffer is narrowed.

[John Kitchin]

It has always bugged me when clicking on an id link does not put the
point on the headline when narrowing is in place. Opening in a new
buffer might  be a nice compromise of preserving the restriction in
place, and getting to the point of interest. I think these are called
indirect buffers
(http://www.gnu.org/software/emacs/manual/html_node/emacs/Indirect-Buffers.html).

Puneeth Chaganti writes:

> Hi Rasmus,
>
> On Fri, Oct 23, 2015 at 8:57 PM, Rasmus <rasmus@gmx.us> wrote:
>> It's not obvious that org should change a—potentially—carefully selected
>> narrowed region.
>
> I agree. But, am I not explicitly asking to jump to the specified
> item. I don't mind the widening, at least when the call is
> interactive. I agree with you when some other code is calling it,
> though.
>
>> Perhaps you could mimic the way org-edit-special works for this case.
>
> You mean, display the entry in a new buffer, and any changes will be
> applied onto the original entry too?

--
Professor John Kitchin
Doherty Hall A207F
Department of Chemical Engineering
Carnegie Mellon University
Pittsburgh, PA 15213
412-268-7803
@johnkitchin
http://kitchingroup.cheme.cmu.edu

Re: [PATCH] org-id-goto doesn't work if buffer is narrowed.

[Rasmus]

Hi Puneeth,

Thanks for the followup.

Puneeth Chaganti <punchagan@gmail.com> writes:

> On Fri, Oct 23, 2015 at 8:57 PM, Rasmus <rasmus@gmx.us> wrote:
>> It's not obvious that org should change a—potentially—carefully selected
>> narrowed region.
>
> I agree. But, am I not explicitly asking to jump to the specified
> item. I don't mind the widening, at least when the call is
> interactive. I agree with you when some other code is calling it,
> though.

I see your point, but I also remember being quite annoyed in the past when
I lost my narrow because of e.g. inserting a footnote.

>> Perhaps you could mimic the way org-edit-special works for this case.
>
> You mean, display the entry in a new buffer, and any changes will be
> applied onto the original entry too?

Yeah, I would prefer that.  Would that work for you or would still prefer
to have your buffer widened?

Thanks,
Rasmus

-- 
I almost cut my hair, it happened just the other day

Re: [PATCH] org-id-goto doesn't work if buffer is narrowed.

[Puneeth Chaganti]

Hi Rasmus,

On Sat, Oct 24, 2015 at 1:52 AM, Rasmus <rasmus@gmx.us> wrote:
> Hi Puneeth,
>
> Thanks for the followup.
>
> Puneeth Chaganti <punchagan@gmail.com> writes:
>
>> On Fri, Oct 23, 2015 at 8:57 PM, Rasmus <rasmus@gmx.us> wrote:
>>> It's not obvious that org should change a—potentially—carefully selected
>>> narrowed region.
>>
>> I agree. But, am I not explicitly asking to jump to the specified
>> item. I don't mind the widening, at least when the call is
>> interactive. I agree with you when some other code is calling it,
>> though.
>
> I see your point, but I also remember being quite annoyed in the past when
> I lost my narrow because of e.g. inserting a footnote.

I see.

>>> Perhaps you could mimic the way org-edit-special works for this case.
>>
>> You mean, display the entry in a new buffer, and any changes will be
>> applied onto the original entry too?
>
> Yeah, I would prefer that.  Would that work for you or would still prefer
> to have your buffer widened?

I agree that widening a buffer that was narrowed on purpose can be
annoying, sometimes. Most times, I think I wouldn't mind the widening.
That said, I'm not quite sure what is the right fix for this.

I find it weird to have a subtly different thing happening depending
on whether or not the target buffer is narrowed -- entry shown in
normal buffer when no narrowing vs. entry shown in a special/indirect
buffer.

Also, given that no other part of org really uses indirect buffers, I
don't know if this function alone should make use of that feature.

Let me know what you think.

-- Puneeth

PS: I've patched my org sources to do indirect buffers for this, and
will try it out for sometime to see how it feels.

Re: [PATCH] org-id-goto doesn't work if buffer is narrowed.

[John Kitchin]

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

> >
> >> On Fri, Oct 23, 2015 at 8:57 PM, Rasmus <rasmus@gmx.us> wrote:
> >>> It's not obvious that org should change a—potentially—carefully
> selected
> >>> narrowed region.
> >>
> >> I agree. But, am I not explicitly asking to jump to the specified
> >> item. I don't mind the widening, at least when the call is
> >> interactive. I agree with you when some other code is calling it,
> >> though.
>

Maybe I am missing something here. I would expect org-id-goto to actually
get to the id entry when it is used independent of narrowing. When used in
a program, I would expect this behavior to be wrapped in save-restriction
type macros, so it wouldn't change your restriction. But when used
interactively, e.g. when I click on a link, I expect the point to end up on
the id entry, with the buffer open in front of me, even if that means
widening. Is there some other expectation that makes sense? I feel like it
is up to me to decide if breaking the restriction is worth visiting the
link, and only by clicking on the link or running an interactive command
makes that happen.

An alternative might be to check if a restriction is in place somehow, and
make org-id-goto not work or warn you somehow if it has to remove the
restriction.

Is it possible to save a restriction in a variable? so that something like
C-c & could restore it?  the save-restriction macro must do something like
that, but the code seems to be hidden in the C-source for me.


> >
> > I see your point, but I also remember being quite annoyed in the past
> when
> > I lost my narrow because of e.g. inserting a footnote.
>
> I see.
>
> >>> Perhaps you could mimic the way org-edit-special works for this case.
> >>
> >> You mean, display the entry in a new buffer, and any changes will be
> >> applied onto the original entry too?
> >
> > Yeah, I would prefer that.  Would that work for you or would still prefer
> > to have your buffer widened?
>
> I agree that widening a buffer that was narrowed on purpose can be
> annoying, sometimes. Most times, I think I wouldn't mind the widening.
> That said, I'm not quite sure what is the right fix for this.
>
> I find it weird to have a subtly different thing happening depending
> on whether or not the target buffer is narrowed -- entry shown in
> normal buffer when no narrowing vs. entry shown in a special/indirect
> buffer.
>
> Also, given that no other part of org really uses indirect buffers, I
> don't know if this function alone should make use of that feature.
>
> Let me know what you think.
>
> -- Puneeth
>
> PS: I've patched my org sources to do indirect buffers for this, and
> will try it out for sometime to see how it feels.
>
>

[-- Attachment #2: Type: text/html, Size: 3607 bytes --]

Re: [PATCH] org-id-goto doesn't work if buffer is narrowed.

[Puneeth Chaganti]

Hi John,

On Sat, Oct 24, 2015 at 5:03 PM, John Kitchin <jkitchin@andrew.cmu.edu> wrote:
>
> Maybe I am missing something here. I would expect org-id-goto to actually
> get to the id entry when it is used independent of narrowing. When used in a
> program, I would expect this behavior to be wrapped in save-restriction type
> macros, so it wouldn't change your restriction. But when used interactively,
> e.g. when I click on a link, I expect the point to end up on the id entry,
> with the buffer open in front of me, even if that means widening. Is there
> some other expectation that makes sense? I feel like it is up to me to
> decide if breaking the restriction is worth visiting the link, and only by
> clicking on the link or running an interactive command makes that happen.

Thanks for spelling it out so clearly. I wasn't thinking clearly,
before.  I completely agree with you that the user should be the one
deciding if its "worth breaking the restriction".  Any calling code
would be saving restriction, if that is the intent.

+1 on simply widening the buffer, with an optional warning/error that
can be turned on or off with a variable.

-- 
Puneeth

Re: [PATCH] org-id-goto doesn't work if buffer is narrowed.

[Nicolas Goaziou]

Hello,

John Kitchin <jkitchin@andrew.cmu.edu> writes:

> Maybe I am missing something here. I would expect org-id-goto to actually
> get to the id entry when it is used independent of narrowing. When used in
> a program, I would expect this behavior to be wrapped in save-restriction
> type macros, so it wouldn't change your restriction. But when used
> interactively, e.g. when I click on a link, I expect the point to end up on
> the id entry, with the buffer open in front of me, even if that means
> widening. Is there some other expectation that makes sense? I feel like it
> is up to me to decide if breaking the restriction is worth visiting the
> link, and only by clicking on the link or running an interactive command
> makes that happen.
>
> An alternative might be to check if a restriction is in place somehow, and
> make org-id-goto not work or warn you somehow if it has to remove the
> restriction.

I agree that we should first widen the buffer unconditionally in order
to find the location. Once the issue is fixed, we can discuss about
a better UI.

Regards,

-- 
Nicolas Goaziou

Re: [PATCH] org-id-goto doesn't work if buffer is narrowed.

[Rasmus]

John Kitchin <jkitchin@andrew.cmu.edu> writes:

> Maybe I am missing something here. I would expect org-id-goto to actually
> get to the id entry when it is used independent of narrowing. When used in
> a program, I would expect this behavior to be wrapped in save-restriction
> type macros, so it wouldn't change your restriction. But when used
> interactively, e.g. when I click on a link, I expect the point to end up on
> the id entry, with the buffer open in front of me, even if that means
> widening. Is there some other expectation that makes sense? I feel like it
> is up to me to decide if breaking the restriction is worth visiting the
> link, and only by clicking on the link or running an interactive command
> makes that happen.

I prefer the behavior of C-c C-c on a footnote in a narrowed buffer.
I.e. throw an error.


> Is it possible to save a restriction in a variable? so that something like
> C-c & could restore it?  the save-restriction macro must do something like
> that, but the code seems to be hidden in the C-source for me.


I thought about that.  I sort of like, but I also think it’s a potentially
big change...

A poor man’s solution might be:

  (when (buffer-narrowed-p) (cons (point-min) (point-max)))

Rasmus

-- 
When the facts change, I change my mind. What do you do, sir?

Re: [PATCH] org-id-goto doesn't work if buffer is narrowed.

[John Kitchin]

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

I like the idea too. I worked out a partial solution for id links here:
http://kitchingroup.cheme.cmu.edu/blog/2015/10/24/Saving-the-current-restriction-and-restoring-it-while-following-links/
using the idea for saving and restoring the restriction.

John

-----------------------------------
Professor John Kitchin
Doherty Hall A207F
Department of Chemical Engineering
Carnegie Mellon University
Pittsburgh, PA 15213
412-268-7803
@johnkitchin
http://kitchingroup.cheme.cmu.edu


On Sat, Oct 24, 2015 at 8:47 AM, Rasmus <rasmus@gmx.us> wrote:

> John Kitchin <jkitchin@andrew.cmu.edu> writes:
>
> > Maybe I am missing something here. I would expect org-id-goto to actually
> > get to the id entry when it is used independent of narrowing. When used
> in
> > a program, I would expect this behavior to be wrapped in save-restriction
> > type macros, so it wouldn't change your restriction. But when used
> > interactively, e.g. when I click on a link, I expect the point to end up
> on
> > the id entry, with the buffer open in front of me, even if that means
> > widening. Is there some other expectation that makes sense? I feel like
> it
> > is up to me to decide if breaking the restriction is worth visiting the
> > link, and only by clicking on the link or running an interactive command
> > makes that happen.
>
> I prefer the behavior of C-c C-c on a footnote in a narrowed buffer.
> I.e. throw an error.
>
>
> > Is it possible to save a restriction in a variable? so that something
> like
> > C-c & could restore it?  the save-restriction macro must do something
> like
> > that, but the code seems to be hidden in the C-source for me.
>
>
> I thought about that.  I sort of like, but I also think it’s a potentially
> big change...
>
> A poor man’s solution might be:
>
>   (when (buffer-narrowed-p) (cons (point-min) (point-max)))
>
> Rasmus
>
> --
> When the facts change, I change my mind. What do you do, sir?
>
>
>

[-- Attachment #2: Type: text/html, Size: 2899 bytes --]

Re: [PATCH] org-id-goto doesn't work if buffer is narrowed.

[Rasmus]

John Kitchin <jkitchin@andrew.cmu.edu> writes:

> I like the idea too. I worked out a partial solution for id links here:
> http://kitchingroup.cheme.cmu.edu/blog/2015/10/24/Saving-the-current-restriction-and-restoring-it-while-following-links/
> using the idea for saving and restoring the restriction.

Thanks, John.

You will probably want to use markers since one might edit stuff
in-between the widening and going back to the narrow.

     http://www.gnu.org/software/emacs/manual/html_node/elisp/Overview-of-Markers.html#Overview-of-Markers

Markers should be cleaned up, which might prove difficult since one might
not go back with C-c &, but you can at least ensure that at most two loose
markers exists (potentially per Org buffer).

Also, I wonder if there exits cases where you would need to store the
buffer as well as the narrow points...  I guess not?

Thanks,
Rasmus

-- 
Slowly unravels in a ball of yarn and the devil collects it

Re: [PATCH] org-id-goto doesn't work if buffer is narrowed.

[John Kitchin]

In my solution I do store the buffer. Maybe for some reason the id is in
another buffer with narrowing, and you want to preserve that narrowing
when you return. I originally did that with a marker, so you could
restore point to, but I decided to take that out. I wasn't aware you
could have dangling markers! I guess you would want to use more global
variables, so there would only be two. and hopefully you don't follow
multiple id links through narrowed buffers in a row!


Rasmus writes:

> John Kitchin <jkitchin@andrew.cmu.edu> writes:
>
>> I like the idea too. I worked out a partial solution for id links here:
>> http://kitchingroup.cheme.cmu.edu/blog/2015/10/24/Saving-the-current-restriction-and-restoring-it-while-following-links/
>> using the idea for saving and restoring the restriction.
>
> Thanks, John.
>
> You will probably want to use markers since one might edit stuff
> in-between the widening and going back to the narrow.
>
>      http://www.gnu.org/software/emacs/manual/html_node/elisp/Overview-of-Markers.html#Overview-of-Markers
>
> Markers should be cleaned up, which might prove difficult since one might
> not go back with C-c &, but you can at least ensure that at most two loose
> markers exists (potentially per Org buffer).
>
> Also, I wonder if there exits cases where you would need to store the
> buffer as well as the narrow points...  I guess not?
>
> Thanks,
> Rasmus

--
Professor John Kitchin
Doherty Hall A207F
Department of Chemical Engineering
Carnegie Mellon University
Pittsburgh, PA 15213
412-268-7803
@johnkitchin
http://kitchingroup.cheme.cmu.edu

Re: [PATCH] org-id-goto doesn't work if buffer is narrowed.

[Rasmus]

Hi Puneeth,

First, I now think your suggestion (to widen) is consistent with the
current behavior of Org.  E.g. C-c C-o on a CUSTOM_IDs link will widen the
buffer.  Also, if ID is outside of the current file, it will follow the
link.  Second, I think the current behavior should be changed, but that is
a separate discussion.

Thus, I think for now your suggestion is the correct one.

However, you need to make sure that it only widens if ID is in the same
buffer and outside of the narrow.  In particular, when ID is in another
file you shouldn’t widen.  When it’s within the scope of the narrow it
shouldn’t widen.  This ALREADY seems to be the case.

However, your patch doesn’t work for me in the following example, starting
from emacs -q, adding /tmp/test.org (with the below content) to my agenda
list and requiring org-id, org-narrow-to-subtree on foo, and then
org-open-at-point on the link

    * foo
    [[id:myid]]  now C-c C-o on link

    * bar
    :PROPERTIES:
    :ID:       myid
    :END:

We can add your patch when it works properly.

> I find it weird to have a subtly different thing happening depending
> on whether or not the target buffer is narrowed -- entry shown in
> normal buffer when no narrowing vs. entry shown in a special/indirect
> buffer.

Widening is the current behavior when you click on a most link in a
narrowed buffer.  Personally, I would rather throw an error.

> Also, given that no other part of org really uses indirect buffers, I
> don't know if this function alone should make use of that feature.

Another option might be to make "C-c &" recall the last narrow (as John
also mentions).  But this is sort of redefining the meaning of C-c &,
which may not be comfortable.

> PS: I've patched my org sources to do indirect buffers for this, and
> will try it out for sometime to see how it feels.

Thanks a lot for your efforts, Puneeth.

Cheers,
Rasmus

-- 
Together we will make the possible totalllly impossible!

Re: [PATCH] org-id-goto doesn't work if buffer is narrowed.

[Puneeth Chaganti]

Hi Rasmus,

On Sat, Oct 24, 2015 at 5:57 PM, Rasmus <rasmus@gmx.us> wrote:
>
> However, you need to make sure that it only widens if ID is in the same
> buffer and outside of the narrow.  In particular, when ID is in another
> file you shouldn’t widen.  When it’s within the scope of the narrow it
> shouldn’t widen.  This ALREADY seems to be the case.

I will fix the patch to widen only when the target location is not
within the narrow. But, I don't understand why we shouldn't be
widening if the target location is in a different buffer. Can you
explain?

> However, your patch doesn’t work for me in the following example, starting
> from emacs -q, adding /tmp/test.org (with the below content) to my agenda
> list and requiring org-id, org-narrow-to-subtree on foo, and then
> org-open-at-point on the link

I had patched `org-id-goto' and looks like clicking on links uses
`org-id-open'.  I will resend a patch.  I wonder if these two
functions can reuse common code.

-- 
Puneeth

Re: [PATCH] org-id-goto doesn't work if buffer is narrowed.

[Puneeth Chaganti]

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

Hi Rasmus,

On Sun, Oct 25, 2015 at 7:54 AM, Puneeth Chaganti <punchagan@gmail.com> wrote:
>
>> However, your patch doesn’t work for me in the following example, starting
>> from emacs -q, adding /tmp/test.org (with the below content) to my agenda
>> list and requiring org-id, org-narrow-to-subtree on foo, and then
>> org-open-at-point on the link
>
> I had patched `org-id-goto' and looks like clicking on links uses
> `org-id-open'.  I will resend a patch.  I wonder if these two
> functions can reuse common code.

Here is a patch that works for the case you describe.

The widening happens even if the target location is in a different
buffer, in this patch.

[-- Attachment #2: 0001-Widen-if-target-id-location-is-not-in-the-narrow.patch --]
[-- Type: text/x-patch, Size: 2997 bytes --]

From ba62042ff37c200d814567a79bcb999aef67581c Mon Sep 17 00:00:00 2001
From: Puneeth Chaganti <punchagan@muse-amuse.in>
Date: Sun, 25 Oct 2015 08:24:09 +0530
Subject: [PATCH] Widen if target id location is not in the narrow.

---
 lisp/org-id.el | 52 ++++++++++++++++++++++++++--------------------------
 1 file changed, 26 insertions(+), 26 deletions(-)

diff --git a/lisp/org-id.el b/lisp/org-id.el
index f86ef22..57d2414 100644
--- a/lisp/org-id.el
+++ b/lisp/org-id.el
@@ -292,13 +292,7 @@ It returns the ID of the entry.  If necessary, the ID is created."
   "Switch to the buffer containing the entry with id ID.
 Move the cursor to that entry in that buffer."
   (interactive "sID: ")
-  (let ((m (org-id-find id 'marker)))
-    (unless m
-      (error "Cannot find entry with ID \"%s\"" id))
-    (org-pop-to-buffer-same-window (marker-buffer m))
-    (goto-char m)
-    (move-marker m nil)
-    (org-show-context)))
+  (org-id-show id 'org-pop-to-buffer-same-window))
 
 ;;;###autoload
 (defun org-id-find (id &optional markerp)
@@ -634,6 +628,22 @@ optional argument MARKERP, return the position as a new marker."
 		(move-marker (make-marker) pos buf)
 	      (cons file pos))))))))
 
+(defun org-id-show (id cmd)
+  "Show an entry with id ID by buffer-switching using CMD."
+  (let ((m (org-id-find id 'marker)))
+    (unless m
+      (error "Cannot find entry with ID \"%s\"" id))
+    (if (not (equal (current-buffer) (marker-buffer m)))
+	(funcall cmd (marker-buffer m)))
+    (when (and (org-buffer-narrowed-p)
+	       (let ((pos (marker-position m)))
+		 (or (< pos (point-min))
+		     (> pos (point-max)))))
+      (widen))
+    (goto-char m)
+    (move-marker m nil)
+    (org-show-context)))
+
 ;; id link type
 
 ;; Calling the following function is hard-coded into `org-store-link',
@@ -659,25 +669,15 @@ optional argument MARKERP, return the position as a new marker."
 (defun org-id-open (id)
   "Go to the entry with id ID."
   (org-mark-ring-push)
-  (let ((m (org-id-find id 'marker))
-	cmd)
-    (unless m
-      (error "Cannot find entry with ID \"%s\"" id))
-    ;; Use a buffer-switching command in analogy to finding files
-    (setq cmd
-	  (or
-	   (cdr
-	    (assq
-	     (cdr (assq 'file org-link-frame-setup))
-	     '((find-file . switch-to-buffer)
-	       (find-file-other-window . switch-to-buffer-other-window)
-	       (find-file-other-frame . switch-to-buffer-other-frame))))
-	   'switch-to-buffer-other-window))
-    (if (not (equal (current-buffer) (marker-buffer m)))
-	(funcall cmd (marker-buffer m)))
-    (goto-char m)
-    (move-marker m nil)
-    (org-show-context)))
+  (let ((cmd (or
+	      (cdr
+	       (assq
+		(cdr (assq 'file org-link-frame-setup))
+		'((find-file . switch-to-buffer)
+		  (find-file-other-window . switch-to-buffer-other-window)
+		  (find-file-other-frame . switch-to-buffer-other-frame))))
+	      'switch-to-buffer-other-window)))
+    (org-id-show id cmd)))
 
 (org-add-link-type "id" 'org-id-open)
 
-- 
2.5.0

Re: [PATCH] org-id-goto doesn't work if buffer is narrowed.

[Nicolas Goaziou]

Hello,

Puneeth Chaganti <punchagan@gmail.com> writes:

> Here is a patch that works for the case you describe.

Thank you. Some comments follow.

> +  (org-id-show id 'org-pop-to-buffer-same-window))

  (org-id-show id #'org-pop-to-buffer-same-window)

> +(defun org-id-show (id cmd)
> +  "Show an entry with id ID by buffer-switching using CMD."

You should explain what is CMD. If it is a function, you should spell
out its signature.

> +  (let ((m (org-id-find id 'marker)))
> +    (unless m
> +      (error "Cannot find entry with ID \"%s\"" id))
> +    (if (not (equal (current-buffer) (marker-buffer m)))
> +	(funcall cmd (marker-buffer m)))

(unless (eq (current-buffer) (marker-buffer m))
 ...)
 
> +    (when (and (org-buffer-narrowed-p)
> +	       (let ((pos (marker-position m)))
> +		 (or (< pos (point-min))
> +		     (> pos (point-max)))))
> +      (widen))

`org-buffer-narrowed-p' is not useful here since you check boundaries
right after its call.

  (when (let ((pos (marker-position m)))
          (or (< pos (point-min))
              (> pos (point-max))))
    (widen))

> +    (goto-char m)
> +    (move-marker m nil)
> +    (org-show-context)))

I think this should be (org-show-context 'link-search) according to
`org-show-context-detail'.

>  ;; id link type
>  
>  ;; Calling the following function is hard-coded into `org-store-link',
> @@ -659,25 +669,15 @@ optional argument MARKERP, return the position as a new marker."
>  (defun org-id-open (id)
>    "Go to the entry with id ID."
>    (org-mark-ring-push)
> -  (let ((m (org-id-find id 'marker))
> -	cmd)
> -    (unless m
> -      (error "Cannot find entry with ID \"%s\"" id))
> -    ;; Use a buffer-switching command in analogy to finding files
> -    (setq cmd
> -	  (or
> -	   (cdr
> -	    (assq
> -	     (cdr (assq 'file org-link-frame-setup))
> -	     '((find-file . switch-to-buffer)
> -	       (find-file-other-window . switch-to-buffer-other-window)
> -	       (find-file-other-frame . switch-to-buffer-other-frame))))
> -	   'switch-to-buffer-other-window))
> -    (if (not (equal (current-buffer) (marker-buffer m)))
> -	(funcall cmd (marker-buffer m)))
> -    (goto-char m)
> -    (move-marker m nil)
> -    (org-show-context)))
> +  (let ((cmd (or
> +	      (cdr
> +	       (assq
> +		(cdr (assq 'file org-link-frame-setup))
> +		'((find-file . switch-to-buffer)
> +		  (find-file-other-window . switch-to-buffer-other-window)
> +		  (find-file-other-frame . switch-to-buffer-other-frame))))
> +	      'switch-to-buffer-other-window)))

  #'switch-to-buffer-other-window

Regards,

-- 
Nicolas Goaziou

Re: [PATCH] org-id-goto doesn't work if buffer is narrowed.

[Puneeth Chaganti]

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

Hi Nicolas,

On Sun, Oct 25, 2015 at 2:08 PM, Nicolas Goaziou <mail@nicolasgoaziou.fr> wrote:
> Hello,
>
> Puneeth Chaganti <punchagan@gmail.com> writes:
>
>> Here is a patch that works for the case you describe.
>
> Thank you. Some comments follow.

Thanks for your careful review and detailed comments.  I've attached
an updated patch.

-- Puneeth

[-- Attachment #2: 0001-Widen-if-target-id-location-is-not-in-the-narrow.patch --]
[-- Type: text/x-patch, Size: 3271 bytes --]

From 5f0746566cc63c37634b20070a50601c5bb990ab Mon Sep 17 00:00:00 2001
From: Puneeth Chaganti <punchagan@muse-amuse.in>
Date: Sun, 25 Oct 2015 14:36:16 +0530
Subject: [PATCH] Widen if target id location is not in the narrow.

If the target location for `org-id-goto' or `org-id-open' is in a
narrowed buffer but not in the narrowed region, the buffer is widened.
---
 lisp/org-id.el | 54 ++++++++++++++++++++++++++++--------------------------
 1 file changed, 28 insertions(+), 26 deletions(-)

diff --git a/lisp/org-id.el b/lisp/org-id.el
index f86ef22..0e8ba75 100644
--- a/lisp/org-id.el
+++ b/lisp/org-id.el
@@ -292,13 +292,7 @@ It returns the ID of the entry.  If necessary, the ID is created."
   "Switch to the buffer containing the entry with id ID.
 Move the cursor to that entry in that buffer."
   (interactive "sID: ")
-  (let ((m (org-id-find id 'marker)))
-    (unless m
-      (error "Cannot find entry with ID \"%s\"" id))
-    (org-pop-to-buffer-same-window (marker-buffer m))
-    (goto-char m)
-    (move-marker m nil)
-    (org-show-context)))
+  (org-id-show id #'org-pop-to-buffer-same-window))
 
 ;;;###autoload
 (defun org-id-find (id &optional markerp)
@@ -634,6 +628,24 @@ optional argument MARKERP, return the position as a new marker."
 		(move-marker (make-marker) pos buf)
 	      (cons file pos))))))))
 
+(defun org-id-show (id cmd)
+  "Show an entry with id ID by buffer-switching using CMD.
+CMD is a function that takes a buffer or a string (buffer name)
+as an argument, which will be used to switch to the buffer
+containing the entry with id ID."
+  (let ((m (org-id-find id 'marker)))
+    (unless m
+      (error "Cannot find entry with ID \"%s\"" id))
+    (unless (equal (current-buffer) (marker-buffer m))
+      (funcall cmd (marker-buffer m)))
+    (when (let ((pos (marker-position m)))
+	    (or (< pos (point-min))
+		(> pos (point-max))))
+      (widen))
+    (goto-char m)
+    (move-marker m nil)
+    (org-show-context 'link-search)))
+
 ;; id link type
 
 ;; Calling the following function is hard-coded into `org-store-link',
@@ -659,25 +671,15 @@ optional argument MARKERP, return the position as a new marker."
 (defun org-id-open (id)
   "Go to the entry with id ID."
   (org-mark-ring-push)
-  (let ((m (org-id-find id 'marker))
-	cmd)
-    (unless m
-      (error "Cannot find entry with ID \"%s\"" id))
-    ;; Use a buffer-switching command in analogy to finding files
-    (setq cmd
-	  (or
-	   (cdr
-	    (assq
-	     (cdr (assq 'file org-link-frame-setup))
-	     '((find-file . switch-to-buffer)
-	       (find-file-other-window . switch-to-buffer-other-window)
-	       (find-file-other-frame . switch-to-buffer-other-frame))))
-	   'switch-to-buffer-other-window))
-    (if (not (equal (current-buffer) (marker-buffer m)))
-	(funcall cmd (marker-buffer m)))
-    (goto-char m)
-    (move-marker m nil)
-    (org-show-context)))
+  (let ((cmd (or
+	      (cdr
+	       (assq
+		(cdr (assq 'file org-link-frame-setup))
+		'((find-file . switch-to-buffer)
+		  (find-file-other-window . switch-to-buffer-other-window)
+		  (find-file-other-frame . switch-to-buffer-other-frame))))
+	      #'switch-to-buffer-other-window)))
+    (org-id-show id cmd)))
 
 (org-add-link-type "id" 'org-id-open)
 
-- 
2.5.0

Re: [PATCH] org-id-goto doesn't work if buffer is narrowed.

[Nicolas Goaziou]

Puneeth Chaganti <punchagan@gmail.com> writes:

> Thanks for your careful review and detailed comments.  I've attached
> an updated patch.

Thank you. I have another suggestion about it.

> +(defun org-id-show (id cmd)
> +  "Show an entry with id ID by buffer-switching using CMD.
> +CMD is a function that takes a buffer or a string (buffer name)
> +as an argument, which will be used to switch to the buffer
> +containing the entry with id ID."
> +  (let ((m (org-id-find id 'marker)))
> +    (unless m
> +      (error "Cannot find entry with ID \"%s\"" id))
> +    (unless (equal (current-buffer) (marker-buffer m))
> +      (funcall cmd (marker-buffer m)))

Nitpick: (eq (current-buffer) (marker-buffer m))
  
> +    (when (let ((pos (marker-position m)))
> +	    (or (< pos (point-min))
> +		(> pos (point-max))))
> +      (widen))
> +    (goto-char m)
> +    (move-marker m nil)
> +    (org-show-context 'link-search)))

If CMD raises an error, you have a dangling marker in the buffer, which
is not a great idea. I suggest to wrap everything into
a `unwind-protect' and add (set-marker m nil) as an unwindform, i.e.,

  (let ((m (org-id-find id 'marker)))
    (unless m (error "Cannot find entry with ID \"%s\"" id))
    (unwind-protect
        (progn
          (unless (eq (current-buffer) (marker-buffer m))
            (funcall cmd (marker-buffer m)))
          (when (let ((pos (marker-position m)))
                  (or (< pos (point-min))
                      (> pos (point-max))))
            (widen))
          (goto-char m)
          (org-show-context 'link-search))
      (set-marker m nil)))

Regards,

Re: [PATCH] org-id-goto doesn't work if buffer is narrowed.

[Puneeth Chaganti]

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

> If CMD raises an error, you have a dangling marker in the buffer, which
> is not a great idea. I suggest to wrap everything into
> a `unwind-protect' and add (set-marker m nil) as an unwindform, i.e.,

Fixed. Thanks!

[-- Attachment #2: 0001-Widen-if-target-id-location-is-not-in-the-narrow.patch --]
[-- Type: text/x-patch, Size: 3371 bytes --]

From 8ec4f777a2e1078e1fadc48a7c91366200a0d37b Mon Sep 17 00:00:00 2001
From: Puneeth Chaganti <punchagan@muse-amuse.in>
Date: Sun, 25 Oct 2015 14:36:16 +0530
Subject: [PATCH] Widen if target id location is not in the narrow.

If the target location for `org-id-goto' or `org-id-open' is in a
narrowed buffer but not in the narrowed region, the buffer is widened.
---
 lisp/org-id.el | 55 +++++++++++++++++++++++++++++--------------------------
 1 file changed, 29 insertions(+), 26 deletions(-)

diff --git a/lisp/org-id.el b/lisp/org-id.el
index f86ef22..73e8f54 100644
--- a/lisp/org-id.el
+++ b/lisp/org-id.el
@@ -292,13 +292,7 @@ It returns the ID of the entry.  If necessary, the ID is created."
   "Switch to the buffer containing the entry with id ID.
 Move the cursor to that entry in that buffer."
   (interactive "sID: ")
-  (let ((m (org-id-find id 'marker)))
-    (unless m
-      (error "Cannot find entry with ID \"%s\"" id))
-    (org-pop-to-buffer-same-window (marker-buffer m))
-    (goto-char m)
-    (move-marker m nil)
-    (org-show-context)))
+  (org-id-show id #'org-pop-to-buffer-same-window))
 
 ;;;###autoload
 (defun org-id-find (id &optional markerp)
@@ -634,6 +628,25 @@ optional argument MARKERP, return the position as a new marker."
 		(move-marker (make-marker) pos buf)
 	      (cons file pos))))))))
 
+(defun org-id-show (id cmd)
+  "Show an entry with id ID by buffer-switching using CMD.
+CMD is a function that takes a buffer or a string (buffer name)
+as an argument, which will be used to switch to the buffer
+containing the entry with id ID."
+  (let ((m (org-id-find id 'marker)))
+    (unless m (error "Cannot find entry with ID \"%s\"" id))
+    (unwind-protect
+        (progn
+          (unless (eq (current-buffer) (marker-buffer m))
+            (funcall cmd (marker-buffer m)))
+          (when (let ((pos (marker-position m)))
+                  (or (< pos (point-min))
+                      (> pos (point-max))))
+            (widen))
+          (goto-char m)
+          (org-show-context 'link-search))
+      (set-marker m nil))))
+
 ;; id link type
 
 ;; Calling the following function is hard-coded into `org-store-link',
@@ -659,25 +672,15 @@ optional argument MARKERP, return the position as a new marker."
 (defun org-id-open (id)
   "Go to the entry with id ID."
   (org-mark-ring-push)
-  (let ((m (org-id-find id 'marker))
-	cmd)
-    (unless m
-      (error "Cannot find entry with ID \"%s\"" id))
-    ;; Use a buffer-switching command in analogy to finding files
-    (setq cmd
-	  (or
-	   (cdr
-	    (assq
-	     (cdr (assq 'file org-link-frame-setup))
-	     '((find-file . switch-to-buffer)
-	       (find-file-other-window . switch-to-buffer-other-window)
-	       (find-file-other-frame . switch-to-buffer-other-frame))))
-	   'switch-to-buffer-other-window))
-    (if (not (equal (current-buffer) (marker-buffer m)))
-	(funcall cmd (marker-buffer m)))
-    (goto-char m)
-    (move-marker m nil)
-    (org-show-context)))
+  (let ((cmd (or
+	      (cdr
+	       (assq
+		(cdr (assq 'file org-link-frame-setup))
+		'((find-file . switch-to-buffer)
+		  (find-file-other-window . switch-to-buffer-other-window)
+		  (find-file-other-frame . switch-to-buffer-other-frame))))
+	      #'switch-to-buffer-other-window)))
+    (org-id-show id cmd)))
 
 (org-add-link-type "id" 'org-id-open)
 
-- 
2.5.0

Re: [PATCH] org-id-goto doesn't work if buffer is narrowed.

[Rasmus]

Hi Puneeth,

Puneeth Chaganti <punchagan@gmail.com> writes:

>> However, you need to make sure that it only widens if ID is in the same
>> buffer and outside of the narrow.  In particular, when ID is in another
>> file you shouldn’t widen.  When it’s within the scope of the narrow it
>> shouldn’t widen.  This ALREADY seems to be the case.
>
> I will fix the patch to widen only when the target location is not
> within the narrow. But, I don't understand why we shouldn't be
> widening if the target location is in a different buffer. Can you
> explain?

Reading your latest patch it seems to be correct.  I mean, if I click
[[id:myid]] in narrowed buffer A and myid is in buffer B, then you
shouldn’t widen in buffer A.  Since you switch buffer there’s no such
issue.

>> However, your patch doesn’t work for me in the following example, starting
>> from emacs -q, adding /tmp/test.org (with the below content) to my agenda
>> list and requiring org-id, org-narrow-to-subtree on foo, and then
>> org-open-at-point on the link
>
> I had patched `org-id-goto' and looks like clicking on links uses
> `org-id-open'.  I will resend a patch.  I wonder if these two
> functions can reuse common code.

The latest patch from

    Date: Sun, 25 Oct 2015 14:36:16 +0530

looks good to me.  Thanks!

Rasmus

-- 
The second rule of Fight Club is: You do not talk about Fight Club

Re: [PATCH] org-id-goto doesn't work if buffer is narrowed.

[Puneeth Chaganti]

On Sun, Oct 25, 2015 at 4:49 PM, Rasmus <rasmus@gmx.us> wrote:
> The latest patch from
>
>     Date: Sun, 25 Oct 2015 14:36:16 +0530
>
> looks good to me.  Thanks!

I guess, this can be merged, then?

Thanks!
Puneeth

Re: [PATCH] org-id-goto doesn't work if buffer is narrowed.

[Matt Lundin]

Rasmus <rasmus@gmx.us> writes:

> Hi Puneeth,
>
> Thanks for your patch.
>
> Puneeth Chaganti <punchagan@gmail.com> writes:
>
>> I've attached a one-line patch to fix `org-id-goto' if the target
>> buffer is narrowed. Let me know if this fix sounds reasonable or if
>> you have other suggestions!
>
> It's not obvious that org should change a—potentially—carefully selected
> narrowed region.

Org mode already does this aggressively and often. For instance, it
irrevocably widens all buffers associated with org-agenda-files when
generating the agenda. (To replicate, try narrowing to a headline in a
buffer and then calling the agenda.) The same is true of follow mode; it
widens a buffer once one moves to a line in the agenda corresponding to
that buffer. In other words, org currently does not go out of its way to
protect user's narrowed regions.

+1 on my part for widening a buffer if it contains a relevant id target.
If I click org-id-goto and the target is in my buffer, I want it to
widen that buffer.

Matt

Re: [PATCH] org-id-goto doesn't work if buffer is narrowed.

[Rasmus]

Hi Matt,

Matt Lundin <mdl@imapmail.org> writes:

>>> I've attached a one-line patch to fix `org-id-goto' if the target
>>> buffer is narrowed. Let me know if this fix sounds reasonable or if
>>> you have other suggestions!
>>
>> It's not obvious that org should change a—potentially—carefully selected
>> narrowed region.
>
> Org mode already does this aggressively and often. For instance, it
> irrevocably widens all buffers associated with org-agenda-files when
> generating the agenda. (To replicate, try narrowing to a headline in a
> buffer and then calling the agenda.) The same is true of follow mode; it
> widens a buffer once one moves to a line in the agenda corresponding to
> that buffer. In other words, org currently does not go out of its way to
> protect user's narrowed regions.
>
> +1 on my part for widening a buffer if it contains a relevant id target.
> If I click org-id-goto and the target is in my buffer, I want it to
> widen that buffer.

It’s not that Org isn’t trying to save narrows, it’s just that you found
another bug (sounds like a missing org-with-wide-buffer to me, but I
haven’t tried to reproduce your example).  It could of course also be that
the same logic is not applied in this case on purpose.

In Org 8.3, you'll (thankfully) not lose your narrow when you click C-c
C-c on a footnote reference outside the current narrow.  Also not when you
insert a new footnote.

Whether it would be better to use an indirect buffer, as John suggests, or
whether it would be possible to utilize the familiar interface of
org-src.el for "remote editing" (or viewing) I don’t know.

Rasmus

-- 
I hear there's rumors on the, uh, Internets. . .