unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#28645: 26.0.50; semantic-ia-fast-jump jumps to a random place in buffer
@ 2017-09-29 13:23 Constantine
  2017-09-29 14:42 ` Dmitry Gutov
  2017-10-03  9:40 ` bug#28645: Status: " Bastian Beischer
  0 siblings, 2 replies; 17+ messages in thread
From: Constantine @ 2017-09-29 13:23 UTC (permalink / raw)
  To: 28645

When declaration is in another buffer, semantic-ia-fast-jump often
jumps to this buffer, but to incorrect line. However if the
declaration is in the same buffer where the command was used, it always
— as far as I've seen — jumps to the correct line.

Steps to reproduce:

1. Create the following 2 files with the following content:
     $ grep -n "" myfunc.h test.cpp
     myfunc.h:1:void myfunc1() {
     myfunc.h:2:}
     myfunc.h:3:void myfunc2() {
     myfunc.h:4:}
     test.cpp:1:#include "myfunc.h"
     test.cpp:2:
     test.cpp:3:int main() {
     test.cpp:4:     myfunc1();
     test.cpp:5:     myfunc2();
     test.cpp:6:}
2. Run emacs -Q test.cpp
3. Enable `M-x semantic-mode`
4. Put cursor into "myfunc2", and type `M-x semantic-ia-fast-jump`
     (you'll see it jumped to the correct declaration at myfunc.h file)
5. Use `C-x b RET` to switch back to test.cpp
6. Put cursor into "myfunc1", and type `M-x semantic-ia-fast-jump`

Result: it jumped to "myfunc2" declaration, not to "myfunc1" as it ought to.

I also want to note that though ATM I am using emacs-git, I've seen the
problem for don't remember how long, just didn't report.





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#28645: 26.0.50; semantic-ia-fast-jump jumps to a random place in buffer
  2017-09-29 13:23 bug#28645: 26.0.50; semantic-ia-fast-jump jumps to a random place in buffer Constantine
@ 2017-09-29 14:42 ` Dmitry Gutov
  2017-09-29 14:57   ` Constantine
  2017-09-29 18:18   ` martin rudalics
  2017-10-03  9:40 ` bug#28645: Status: " Bastian Beischer
  1 sibling, 2 replies; 17+ messages in thread
From: Dmitry Gutov @ 2017-09-29 14:42 UTC (permalink / raw)
  To: Constantine, 28645

On 9/29/17 3:23 PM, Constantine wrote:
'> Result: it jumped to "myfunc2" declaration, not to "myfunc1" as it ought
> to.
> 
> I also want to note that though ATM I am using emacs-git, I've seen the
> problem for don't remember how long, just didn't report.

Thanks for the report. You appear to have fallen victim of our windowing 
improvements. I'm not sure this is the correct fix (Martin?), but here's 
a patch that seems to correct the behavior.

diff --git a/lisp/cedet/semantic/ia.el b/lisp/cedet/semantic/ia.el
index d4201fcf51..0692d03aca 100644
--- a/lisp/cedet/semantic/ia.el
+++ b/lisp/cedet/semantic/ia.el
@@ -322,7 +322,8 @@ semantic-ia--fast-jump-helper
    (semantic-go-to-tag dest)
    ;; 3) go-to-tag doesn't switch the buffer in the current window,
    ;;    so it is like find-file-noselect.  Bring it forward.
-  (switch-to-buffer (current-buffer))
+  (let (switch-to-buffer-preserve-window-point)
+    (switch-to-buffer (current-buffer)))
    ;; 4) Fancy pulsing.
    (pulse-momentary-highlight-one-line (point))
    )





^ permalink raw reply related	[flat|nested] 17+ messages in thread

* bug#28645: 26.0.50; semantic-ia-fast-jump jumps to a random place in buffer
  2017-09-29 14:42 ` Dmitry Gutov
@ 2017-09-29 14:57   ` Constantine
  2017-09-29 18:18   ` martin rudalics
  1 sibling, 0 replies; 17+ messages in thread
From: Constantine @ 2017-09-29 14:57 UTC (permalink / raw)
  To: Dmitry Gutov, 28645

On 29.09.2017 17:42, Dmitry Gutov wrote:
> On 9/29/17 3:23 PM, Constantine wrote:
> '> Result: it jumped to "myfunc2" declaration, not to "myfunc1" as it ought
>> to.
>>
>> I also want to note that though ATM I am using emacs-git, I've seen the
>> problem for don't remember how long, just didn't report.
> 
> Thanks for the report. You appear to have fallen victim of our windowing 
> improvements. I'm not sure this is the correct fix (Martin?), but here's 
> a patch that seems to correct the behavior.
> 
> diff --git a/lisp/cedet/semantic/ia.el b/lisp/cedet/semantic/ia.el
> index d4201fcf51..0692d03aca 100644
> --- a/lisp/cedet/semantic/ia.el
> +++ b/lisp/cedet/semantic/ia.el
> @@ -322,7 +322,8 @@ semantic-ia--fast-jump-helper
>     (semantic-go-to-tag dest)
>     ;; 3) go-to-tag doesn't switch the buffer in the current window,
>     ;;    so it is like find-file-noselect.  Bring it forward.
> -  (switch-to-buffer (current-buffer))
> +  (let (switch-to-buffer-preserve-window-point)
> +    (switch-to-buffer (current-buffer)))
>     ;; 4) Fancy pulsing.
>     (pulse-momentary-highlight-one-line (point))
>     )

Yay, thank you, works for me!

Tested-by: Constantine Kharlamov <Hi-Angel@yandex.ru>





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#28645: 26.0.50; semantic-ia-fast-jump jumps to a random place in buffer
  2017-09-29 14:42 ` Dmitry Gutov
  2017-09-29 14:57   ` Constantine
@ 2017-09-29 18:18   ` martin rudalics
  2017-09-30 14:19     ` Dmitry Gutov
  1 sibling, 1 reply; 17+ messages in thread
From: martin rudalics @ 2017-09-29 18:18 UTC (permalink / raw)
  To: Dmitry Gutov, Constantine, 28645

 > +  (let (switch-to-buffer-preserve-window-point)
 > +    (switch-to-buffer (current-buffer)))

I think (pop-to-buffer-same-window (current-buffer)) should be used
instead.

martin





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#28645: 26.0.50; semantic-ia-fast-jump jumps to a random place in buffer
  2017-09-29 18:18   ` martin rudalics
@ 2017-09-30 14:19     ` Dmitry Gutov
  0 siblings, 0 replies; 17+ messages in thread
From: Dmitry Gutov @ 2017-09-30 14:19 UTC (permalink / raw)
  To: martin rudalics, Constantine, 28645-done

On 9/29/17 8:18 PM, martin rudalics wrote:
>  > +  (let (switch-to-buffer-preserve-window-point)
>  > +    (switch-to-buffer (current-buffer)))
> 
> I think (pop-to-buffer-same-window (current-buffer)) should be used
> instead.

Done.

Thanks all!





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#28645: Status: 26.0.50; semantic-ia-fast-jump jumps to a random place in buffer
  2017-09-29 13:23 bug#28645: 26.0.50; semantic-ia-fast-jump jumps to a random place in buffer Constantine
  2017-09-29 14:42 ` Dmitry Gutov
@ 2017-10-03  9:40 ` Bastian Beischer
  2017-10-04  9:03   ` martin rudalics
  2017-10-15 23:06   ` Dmitry Gutov
  1 sibling, 2 replies; 17+ messages in thread
From: Bastian Beischer @ 2017-10-03  9:40 UTC (permalink / raw)
  To: bug#28645; +Cc: dgutov

Hey,

Thanks for the fix in semantic-ia-fast-jump Dmitry and Martin. However,
I think there are more places which need to be fixed in CEDET. A quick
search for switch-to-buffer shows these in lisp/cedet:

$ grep -n -e 'switch-to-buffer' lisp/cedet
./semantic/decorate/include.el:470:     (switch-to-buffer (get-file-buffer file)))
./semantic/imenu.el:180:                  (switch-to-buffer ob))
./semantic/debug.el:156:      (switch-to-buffer (oref iface data-buffer))
./semantic/debug.el:160:  (switch-to-buffer (oref iface parser-buffer))
./semantic/debug.el:166:  (switch-to-buffer (oref iface source-buffer))
./semantic/debug.el:453:      (switch-to-buffer buf))
./semantic/debug.el:467:      (switch-to-buffer buf))
./semantic/bovine/c.el:980:     (switch-to-buffer (get-buffer-create "*MODE HACK TEST*"))
./semantic/bovine/c.el:992:    (switch-to-buffer-other-window
./semantic/complete.el:1533:    (switch-to-buffer-other-window buf t)
./semantic/complete.el:2123:      (switch-to-buffer (current-buffer))
./semantic/symref/list.el:182:    (switch-to-buffer-other-window buff)
./semantic/symref/list.el:314:    (switch-to-buffer-other-window buff)
./semantic/symref/list.el:328:    (switch-to-buffer-other-window buff)
./semantic/symref/list.el:344:    (switch-to-buffer-other-window buff)
./semantic/symref/list.el:420:    (switch-to-buffer-other-window (semantic-tag-buffer tag))
./semantic/symref/list.el:440:              (switch-to-buffer (current-buffer))
./semantic/sb.el:320:    (switch-to-buffer (current-buffer))
./semantic/analyze/refs.el:351:    (switch-to-buffer (current-buffer))
./semantic/mru-bookmark.el:116:    (switch-to-buffer (current-buffer))
./semantic/texi.el:630:;;       (switch-to-buffer docbuff))
./semantic/texi.el:675:   (switch-to-buffer (semantic-tag-buffer (car tags)))
./semantic/senator.el:533:      (switch-to-buffer (current-buffer))
./semantic/senator.el:537:      (switch-to-buffer result)
./semantic/senator.el:741:                              (switch-to-buffer (semantic-tag-buffer v))
./semantic.el:1081: `global-semantic-mru-bookmark-mode'   - Provide `switch-to-buffer'-like
./srecode/getset.el:263:    (switch-to-buffer (current-buffer))
./ede/custom.el:103:    (switch-to-buffer (get-buffer-create "*EDE sort targets*"))
./ede/shell.el:44:      (switch-to-buffer-other-window buff t))

I was able to reproduce the bug in "semantic-complete-jump" and
"semantic-analyze-proto-impl-toggle", for example.

I would also be grateful for a little bit of background information. At
which point did it become necessary to use 'pop-to-buffer' instead of
'switch-to-buffer'? Apparently 'semantic-ia-fast-jump' et al worked fine
in older emacs versions.

When is it ok to use 'switch-to-buffer'? There are numerous occurences
of it throughout emacs...

Thanks
Bastian





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#28645: Status: 26.0.50; semantic-ia-fast-jump jumps to a random place in buffer
  2017-10-03  9:40 ` bug#28645: Status: " Bastian Beischer
@ 2017-10-04  9:03   ` martin rudalics
  2017-10-04 11:11     ` Bastian Beischer
  2017-10-15 23:06   ` Dmitry Gutov
  1 sibling, 1 reply; 17+ messages in thread
From: martin rudalics @ 2017-10-04  9:03 UTC (permalink / raw)
  To: Bastian Beischer, bug#28645; +Cc: dgutov

 > I would also be grateful for a little bit of background information. At
 > which point did it become necessary to use 'pop-to-buffer' instead of
 > 'switch-to-buffer'? Apparently 'semantic-ia-fast-jump' et al worked fine
 > in older emacs versions.
 >
 > When is it ok to use 'switch-to-buffer'? There are numerous occurences
 > of it throughout emacs...

Consider the following scenario:

(1) ‘switch-to-buffer-preserve-window-point’ is t (it is so by default).

(2) An application explicitly sets point in a buffer that is currently
     not displayed in the selected window to a value different from the
     value of ‘window-point’ of that window at the last time this buffer
     was displayed there.

(3) The application wants to switch to the buffer in the selected window
     with ‘window-point’ at the position of point assigned in (2).

If the application used ‘switch-to-buffer’ in (3), then ‘window-point’
will be reset to the old location of ‘window-point’ which is obviously
not what the application wants.  So to get what it wants, an application
either has to bind ‘switch-to-buffer-preserve-window-point’ to nil
around the ‘switch-to-buffer’ call or use ‘pop-to-buffer-same-window’
(‘display-buffer-same-window’ if the window may remain unselected)
instead.

IMHO applications should never call ‘switch-to-buffer’.  They should
either call ‘pop-to-buffer’ - if the user is supposed to now continue
working in the window showing the buffer - and ‘display-buffer’ in all
other cases.  Both allow the user to control how to display the buffer.

Only if there is a clear preference that the buffer should be shown in
the _selected_ window, the ‘-same-window’ functions should be called.
And programmers should be aware that at the time they want to show a
buffer in the selected window, that window might be the minibuffer
window or a window dedicated to some other buffer.

martin






^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#28645: Status: 26.0.50; semantic-ia-fast-jump jumps to a random place in buffer
  2017-10-04  9:03   ` martin rudalics
@ 2017-10-04 11:11     ` Bastian Beischer
  2017-10-05  8:09       ` martin rudalics
  0 siblings, 1 reply; 17+ messages in thread
From: Bastian Beischer @ 2017-10-04 11:11 UTC (permalink / raw)
  To: martin rudalics; +Cc: bug#28645, Brief Busters

Hey Martin,

On Wed, Oct 4, 2017 at 11:03 AM, martin rudalics <rudalics@gmx.at> wrote:
>> I would also be grateful for a little bit of background information. At
>> which point did it become necessary to use 'pop-to-buffer' instead of
>> 'switch-to-buffer'? Apparently 'semantic-ia-fast-jump' et al worked fine
>> in older emacs versions.
>>
>> When is it ok to use 'switch-to-buffer'? There are numerous occurences
>> of it throughout emacs...
>
> Consider the following scenario:
>
> (1) ‘switch-to-buffer-preserve-window-point’ is t (it is so by default).
>
> (2) An application explicitly sets point in a buffer that is currently
>     not displayed in the selected window to a value different from the
>     value of ‘window-point’ of that window at the last time this buffer
>     was displayed there.
>
> (3) The application wants to switch to the buffer in the selected window
>     with ‘window-point’ at the position of point assigned in (2).
>
> If the application used ‘switch-to-buffer’ in (3), then ‘window-point’
> will be reset to the old location of ‘window-point’ which is obviously
> not what the application wants.  So to get what it wants, an application
> either has to bind ‘switch-to-buffer-preserve-window-point’ to nil
> around the ‘switch-to-buffer’ call or use ‘pop-to-buffer-same-window’
> (‘display-buffer-same-window’ if the window may remain unselected)
> instead.
>
> IMHO applications should never call ‘switch-to-buffer’.  They should
> either call ‘pop-to-buffer’ - if the user is supposed to now continue
> working in the window showing the buffer - and ‘display-buffer’ in all
> other cases.  Both allow the user to control how to display the buffer.
>
> Only if there is a clear preference that the buffer should be shown in
> the _selected_ window, the ‘-same-window’ functions should be called.
> And programmers should be aware that at the time they want to show a
> buffer in the selected window, that window might be the minibuffer
> window or a window dedicated to some other buffer.

I understand. Then this must mean that the change in behavior in CEDET
was triggered with this commit:

commit ee297210cffb9e8d05912686a39fa158414ba050
Author: Mark Oteiza <mvoteiza@udel.edu>
Date:   Thu May 26 21:47:18 2016 -0400

   Preserve buffer point in windows by default (Bug#4041).

   * doc/lispref/windows.texi: Mention new default.
   * etc/NEWS: Mention new default.
   * lisp/window.el (switch-to-buffer-preserve-window-point): Default to t.

which is part of master and emacs-26 but was not part of any previous releases.

I also understand your other arguments. But the question is: While
your recommendation makes sense, there clearly still is a lot of code
which uses switch-to-buffer without binding
switch-to-buffer-preserve-window-point to nil and it wasn't fixed when
this variable's default was changed. This is true in lisp code shipped
in emacs and it is probably also true for lots of third party code in
the wild. Who is going to fix all this code? And if it turns out that
the fixing all this code is too difficult / impossible, is it
justified to fix bug #4041 at the cost of causing numerous other bugs
(which arguably are due to misuse of switch-to-buffer, but they will
have to be fixed either way)?

>
> martin
>

Cheers and thanks again for your answer.
Bastian





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#28645: Status: 26.0.50; semantic-ia-fast-jump jumps to a random place in buffer
  2017-10-04 11:11     ` Bastian Beischer
@ 2017-10-05  8:09       ` martin rudalics
  0 siblings, 0 replies; 17+ messages in thread
From: martin rudalics @ 2017-10-05  8:09 UTC (permalink / raw)
  To: Bastian Beischer; +Cc: bug#28645, Brief Busters

 > I understand. Then this must mean that the change in behavior in CEDET
 > was triggered with this commit:
 >
 > commit ee297210cffb9e8d05912686a39fa158414ba050
 > Author: Mark Oteiza <mvoteiza@udel.edu>
 > Date:   Thu May 26 21:47:18 2016 -0400

Right.

 > I also understand your other arguments. But the question is: While
 > your recommendation makes sense, there clearly still is a lot of code
 > which uses switch-to-buffer without binding
 > switch-to-buffer-preserve-window-point to nil and it wasn't fixed when
 > this variable's default was changed. This is true in lisp code shipped
 > in emacs and it is probably also true for lots of third party code in
 > the wild. Who is going to fix all this code? And if it turns out that
 > the fixing all this code is too difficult / impossible, is it
 > justified to fix bug #4041 at the cost of causing numerous other bugs
 > (which arguably are due to misuse of switch-to-buffer, but they will
 > have to be fixed either way)?

This would have the banal consequence that all users who then set
‘switch-to-buffer-preserve-window-point’ to a non-nil value would have
to live with the wrong behavior forever: Nobody would care about fixing
it because "the default" DTRT already.

martin






^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#28645: Status: 26.0.50; semantic-ia-fast-jump jumps to a random place in buffer
  2017-10-03  9:40 ` bug#28645: Status: " Bastian Beischer
  2017-10-04  9:03   ` martin rudalics
@ 2017-10-15 23:06   ` Dmitry Gutov
  2017-10-16 10:21     ` Bastian Beischer
  1 sibling, 1 reply; 17+ messages in thread
From: Dmitry Gutov @ 2017-10-15 23:06 UTC (permalink / raw)
  To: Bastian Beischer, bug#28645

Hey Bastian,

On 10/3/17 12:40 PM, Bastian Beischer wrote:

> $ grep -n -e 'switch-to-buffer' lisp/cedet
> ./semantic/decorate/include.el:470:     (switch-to-buffer (get-file-buffer file)))
> ./semantic/imenu.el:180:                  (switch-to-buffer ob))
> ./semantic/debug.el:156:      (switch-to-buffer (oref iface data-buffer))
> ./semantic/debug.el:160:  (switch-to-buffer (oref iface parser-buffer))
> ./semantic/debug.el:166:  (switch-to-buffer (oref iface source-buffer))
> ./semantic/debug.el:453:      (switch-to-buffer buf))
> ./semantic/debug.el:467:      (switch-to-buffer buf))
> ./semantic/bovine/c.el:980:     (switch-to-buffer (get-buffer-create "*MODE HACK TEST*"))
> ./semantic/bovine/c.el:992:    (switch-to-buffer-other-window
> ./semantic/complete.el:1533:    (switch-to-buffer-other-window buf t)
> ./semantic/complete.el:2123:      (switch-to-buffer (current-buffer))
> ./semantic/symref/list.el:182:    (switch-to-buffer-other-window buff)
> ./semantic/symref/list.el:314:    (switch-to-buffer-other-window buff)
> ./semantic/symref/list.el:328:    (switch-to-buffer-other-window buff)
> ./semantic/symref/list.el:344:    (switch-to-buffer-other-window buff)
> ./semantic/symref/list.el:420:    (switch-to-buffer-other-window (semantic-tag-buffer tag))
> ./semantic/symref/list.el:440:              (switch-to-buffer (current-buffer))
> ./semantic/sb.el:320:    (switch-to-buffer (current-buffer))
> ./semantic/analyze/refs.el:351:    (switch-to-buffer (current-buffer))
> ./semantic/mru-bookmark.el:116:    (switch-to-buffer (current-buffer))
> ./semantic/texi.el:630:;;       (switch-to-buffer docbuff))
> ./semantic/texi.el:675:   (switch-to-buffer (semantic-tag-buffer (car tags)))
> ./semantic/senator.el:533:      (switch-to-buffer (current-buffer))
> ./semantic/senator.el:537:      (switch-to-buffer result)
> ./semantic/senator.el:741:                              (switch-to-buffer (semantic-tag-buffer v))
> ./semantic.el:1081: `global-semantic-mru-bookmark-mode'   - Provide `switch-to-buffer'-like
> ./srecode/getset.el:263:    (switch-to-buffer (current-buffer))
> ./ede/custom.el:103:    (switch-to-buffer (get-buffer-create "*EDE sort targets*"))
> ./ede/shell.el:44:      (switch-to-buffer-other-window buff t))
> 
> I was able to reproduce the bug in "semantic-complete-jump" and
> "semantic-analyze-proto-impl-toggle", for example.

With Martin's explanation, I guess we should replace all of these uses 
with pop-to-buffer-same-window.

Could you try out this kind of change and see if it fixes the problems 
you are still seeing, without introducing any obvious new ones?





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#28645: Status: 26.0.50; semantic-ia-fast-jump jumps to a random place in buffer
  2017-10-15 23:06   ` Dmitry Gutov
@ 2017-10-16 10:21     ` Bastian Beischer
  2017-10-17  8:58       ` martin rudalics
  2017-10-17 10:44       ` Dmitry Gutov
  0 siblings, 2 replies; 17+ messages in thread
From: Bastian Beischer @ 2017-10-16 10:21 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: bug#28645

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

Hey Dmitry,

On Mon, Oct 16, 2017 at 1:06 AM, Dmitry Gutov <dgutov@yandex.ru> wrote:

> Hey Bastian,
>
> On 10/3/17 12:40 PM, Bastian Beischer wrote:
>
> $ grep -n -e 'switch-to-buffer' lisp/cedet
>> ./semantic/decorate/include.el:470:     (switch-to-buffer
>> (get-file-buffer file)))
>> ./semantic/imenu.el:180:                  (switch-to-buffer ob))
>> ./semantic/debug.el:156:      (switch-to-buffer (oref iface data-buffer))
>> ./semantic/debug.el:160:  (switch-to-buffer (oref iface parser-buffer))
>> ./semantic/debug.el:166:  (switch-to-buffer (oref iface source-buffer))
>> ./semantic/debug.el:453:      (switch-to-buffer buf))
>> ./semantic/debug.el:467:      (switch-to-buffer buf))
>> ./semantic/bovine/c.el:980:     (switch-to-buffer (get-buffer-create
>> "*MODE HACK TEST*"))
>> ./semantic/bovine/c.el:992:    (switch-to-buffer-other-window
>> ./semantic/complete.el:1533:    (switch-to-buffer-other-window buf t)
>> ./semantic/complete.el:2123:      (switch-to-buffer (current-buffer))
>> ./semantic/symref/list.el:182:    (switch-to-buffer-other-window buff)
>> ./semantic/symref/list.el:314:    (switch-to-buffer-other-window buff)
>> ./semantic/symref/list.el:328:    (switch-to-buffer-other-window buff)
>> ./semantic/symref/list.el:344:    (switch-to-buffer-other-window buff)
>> ./semantic/symref/list.el:420:    (switch-to-buffer-other-window
>> (semantic-tag-buffer tag))
>> ./semantic/symref/list.el:440:              (switch-to-buffer
>> (current-buffer))
>> ./semantic/sb.el:320:    (switch-to-buffer (current-buffer))
>> ./semantic/analyze/refs.el:351:    (switch-to-buffer (current-buffer))
>> ./semantic/mru-bookmark.el:116:    (switch-to-buffer (current-buffer))
>> ./semantic/texi.el:630:;;       (switch-to-buffer docbuff))
>> ./semantic/texi.el:675:   (switch-to-buffer (semantic-tag-buffer (car
>> tags)))
>> ./semantic/senator.el:533:      (switch-to-buffer (current-buffer))
>> ./semantic/senator.el:537:      (switch-to-buffer result)
>> ./semantic/senator.el:741:                              (switch-to-buffer
>> (semantic-tag-buffer v))
>> ./semantic.el:1081: `global-semantic-mru-bookmark-mode'   - Provide
>> `switch-to-buffer'-like
>> ./srecode/getset.el:263:    (switch-to-buffer (current-buffer))
>> ./ede/custom.el:103:    (switch-to-buffer (get-buffer-create "*EDE sort
>> targets*"))
>> ./ede/shell.el:44:      (switch-to-buffer-other-window buff t))
>>
>> I was able to reproduce the bug in "semantic-complete-jump" and
>> "semantic-analyze-proto-impl-toggle", for example.
>>
>
> With Martin's explanation, I guess we should replace all of these uses
> with pop-to-buffer-same-window.
>
> Could you try out this kind of change and see if it fixes the problems you
> are still seeing, without introducing any obvious new ones?
>

​I replaced ​switch-to-buffer with pop-to-buffer-same-window in those
functions which I'm using regularly and it fixes the problem. The CEDET
functions where I made the replacements and checked them are:

semantic-analyze-proto-impl-toggle
semantic-complete-jump
semantic-decoration-include-visit
semantic-ia--fast-jump-helper
semantic-mrub-vist
senator-jump
senator-jump-regexp
​senator-go-to-up-reference

​I didn't make any other replacements because:

a) For some of them I'm not sure if 'pop-to-buffer-same-window' or
'pop-to-buffer' ​should be used.
b) I'm not using any of the other functions with matches so I wouldn't be
able to test the change.

Also, searching for 'switch-to-buffer' in non-CEDET related lisp files in
the emacs sources reveals more places which need to be fixed, but I didn't
go that far. However, probably somebody (tm) should check them all
one-by-one :-(

I also think that the advice not to use switch-to-buffer and instead use
pop-to-buffer(-same-window) should get a prominent entry in NEWS so that
third-parties can check their code and make the changes there, too. For
example, SLIME has this open issue:

https://github.com/slime/slime/issues/391
https://lists.gnu.org/archive/html/emacs-devel/2017-06/msg00227.html

and others are probably also affected.

Cheers
Bastian

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#28645: Status: 26.0.50; semantic-ia-fast-jump jumps to a random place in buffer
  2017-10-16 10:21     ` Bastian Beischer
@ 2017-10-17  8:58       ` martin rudalics
  2017-10-17 10:44       ` Dmitry Gutov
  1 sibling, 0 replies; 17+ messages in thread
From: martin rudalics @ 2017-10-17  8:58 UTC (permalink / raw)
  To: Bastian Beischer, Dmitry Gutov; +Cc: bug#28645

 > I also think that the advice not to use switch-to-buffer and instead use
 > pop-to-buffer(-same-window) should get a prominent entry in NEWS so that
 > third-parties can check their code and make the changes there, too.

I added a sentence to that sense.  If more is needed please tell me.

martin





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#28645: Status: 26.0.50; semantic-ia-fast-jump jumps to a random place in buffer
  2017-10-16 10:21     ` Bastian Beischer
  2017-10-17  8:58       ` martin rudalics
@ 2017-10-17 10:44       ` Dmitry Gutov
  2017-10-17 12:30         ` Bastian Beischer
  1 sibling, 1 reply; 17+ messages in thread
From: Dmitry Gutov @ 2017-10-17 10:44 UTC (permalink / raw)
  To: Bastian Beischer; +Cc: bug#28645

On 10/16/17 1:21 PM, Bastian Beischer wrote:

> ​I replaced ​switch-to-buffer with pop-to-buffer-same-window in those 
> functions which I'm using regularly and it fixes the problem. The CEDET 
> functions where I made the replacements and checked them are:
> 
> semantic-analyze-proto-impl-toggle
> semantic-complete-jump
> semantic-decoration-include-visit
> semantic-ia--fast-jump-helper
> semantic-mrub-vist
> senator-jump
> senator-jump-regexp
> ​senator-go-to-up-reference
> 
> ​I didn't make any other replacements because:
> 
> a) For some of them I'm not sure if 'pop-to-buffer-same-window' or 
> 'pop-to-buffer' ​should be used.
> b) I'm not using any of the other functions with matches so I wouldn't 
> be able to test the change.
> 
> Also, searching for 'switch-to-buffer' in non-CEDET related lisp files 
> in the emacs sources reveals more places which need to be fixed, but I 
> didn't go that far. However, probably somebody (tm) should check them 
> all one-by-one :-(

Could you send a patch with the replacements you did make?





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#28645: Status: 26.0.50; semantic-ia-fast-jump jumps to a random place in buffer
  2017-10-17 10:44       ` Dmitry Gutov
@ 2017-10-17 12:30         ` Bastian Beischer
  2017-10-18 23:46           ` Dmitry Gutov
  0 siblings, 1 reply; 17+ messages in thread
From: Bastian Beischer @ 2017-10-17 12:30 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: bug#28645

Dmitry Gutov <dgutov@yandex.ru> writes:

> On 10/16/17 1:21 PM, Bastian Beischer wrote:
>
>> ​I replaced ​switch-to-buffer with pop-to-buffer-same-window in those functions
>> which I'm using regularly and it fixes the problem. The CEDET functions where
>> I made the replacements and checked them are:
>>
>> semantic-analyze-proto-impl-toggle
>> semantic-complete-jump
>> semantic-decoration-include-visit
>> semantic-ia--fast-jump-helper
>> semantic-mrub-vist
>> senator-jump
>> senator-jump-regexp
>> ​senator-go-to-up-reference
>>

It turns out that senator-jump and senator-jump-regexp don't exist in
emacs but only in upstream CEDET. So no need to change these.

>> ​I didn't make any other replacements because:
>>
>> a) For some of them I'm not sure if 'pop-to-buffer-same-window' or
>> 'pop-to-buffer' ​should be used.
>> b) I'm not using any of the other functions with matches so I wouldn't be able
>> to test the change.
>>
>> Also, searching for 'switch-to-buffer' in non-CEDET related lisp files in the
>> emacs sources reveals more places which need to be fixed, but I didn't go that
>> far. However, probably somebody (tm) should check them all one-by-one :-(
>
> Could you send a patch with the replacements you did make?

Sure:

diff --git a/lisp/cedet/semantic/analyze/refs.el b/lisp/cedet/semantic/analyze/refs.el
index 55fcd83043..a58479f505 100644
--- a/lisp/cedet/semantic/analyze/refs.el
+++ b/lisp/cedet/semantic/analyze/refs.el
@@ -348,7 +348,7 @@ Only works for tags in the global namespace."
 
     (push-mark)
     (semantic-go-to-tag target)
-    (switch-to-buffer (current-buffer))
+    (pop-to-buffer-same-windown (current-buffer))
     (semantic-momentary-highlight-tag target))
   )
 
diff --git a/lisp/cedet/semantic/complete.el b/lisp/cedet/semantic/complete.el
index ff8e61e54d..325ca1f441 100644
--- a/lisp/cedet/semantic/complete.el
+++ b/lisp/cedet/semantic/complete.el
@@ -2120,7 +2120,7 @@ completion works."
     (when (semantic-tag-p tag)
       (push-mark)
       (semantic-go-to-tag tag)
-      (switch-to-buffer (current-buffer))
+      (pop-to-buffer-same-window (current-buffer))
       (semantic-momentary-highlight-tag tag)
       (message "%S: %s "
               (semantic-tag-class tag)
diff --git a/lisp/cedet/semantic/decorate/include.el b/lisp/cedet/semantic/decorate/include.el
index 6876e5f3a4..4a86b9e4ee 100644
--- a/lisp/cedet/semantic/decorate/include.el
+++ b/lisp/cedet/semantic/decorate/include.el
@@ -467,7 +467,7 @@ its contents.
        (error "Could not location include %s"
               (semantic-tag-name tag)))
        ((get-file-buffer file)
-       (switch-to-buffer (get-file-buffer file)))
+       (pop-to-buffer-same-window (get-file-buffer file)))
        ((stringp file)
        (find-file file))
        ))))
diff --git a/lisp/cedet/semantic/mru-bookmark.el b/lisp/cedet/semantic/mru-bookmark.el
index 5fa58e08ea..24863de01b 100644
--- a/lisp/cedet/semantic/mru-bookmark.el
+++ b/lisp/cedet/semantic/mru-bookmark.el
@@ -113,7 +113,7 @@ Uses `semantic-go-to-tag' and highlighting."
          (forward-char o))
       (error nil))
     ;; make it visible
-    (switch-to-buffer (current-buffer))
+    (pop-to-buffer-same-window (current-buffer))
     (semantic-momentary-highlight-tag tag)
     ))
 
diff --git a/lisp/cedet/semantic/senator.el b/lisp/cedet/semantic/senator.el
index e86658628b..d37345aa2d 100644
--- a/lisp/cedet/semantic/senator.el
+++ b/lisp/cedet/semantic/senator.el
@@ -530,11 +530,11 @@ Some tags such as includes have other reference features."
        ;; A tag
        ((semantic-tag-p result)
        (semantic-go-to-tag result)
-       (switch-to-buffer (current-buffer))
+       (pop-to-buffer-same-window (current-buffer))
        (semantic-momentary-highlight-tag result))
        ;; Buffers
        ((bufferp result)
-       (switch-to-buffer result)
+       (pop-to-buffer-same-window result)
        (pulse-momentary-highlight-one-line (point)))
        ;; Files
        ((and (stringp result) (file-exists-p result))





^ permalink raw reply related	[flat|nested] 17+ messages in thread

* bug#28645: Status: 26.0.50; semantic-ia-fast-jump jumps to a random place in buffer
  2017-10-17 12:30         ` Bastian Beischer
@ 2017-10-18 23:46           ` Dmitry Gutov
  2017-10-19  9:41             ` Bastian Beischer
  0 siblings, 1 reply; 17+ messages in thread
From: Dmitry Gutov @ 2017-10-18 23:46 UTC (permalink / raw)
  To: Bastian Beischer; +Cc: bug#28645

On 10/17/17 3:30 PM, Bastian Beischer wrote:
>> Could you send a patch with the replacements you did make?
> 
> Sure:

Thanks! I've pushed it to emacs-26 now.





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#28645: Status: 26.0.50; semantic-ia-fast-jump jumps to a random place in buffer
  2017-10-18 23:46           ` Dmitry Gutov
@ 2017-10-19  9:41             ` Bastian Beischer
  2017-10-19 10:03               ` Dmitry Gutov
  0 siblings, 1 reply; 17+ messages in thread
From: Bastian Beischer @ 2017-10-19  9:41 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: bug#28645

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

On Thu, Oct 19, 2017 at 1:46 AM, Dmitry Gutov <dgutov@yandex.ru> wrote:
> On 10/17/17 3:30 PM, Bastian Beischer wrote:
>>>
>>> Could you send a patch with the replacements you did make?
>>
>>
>> Sure:
>
>
> Thanks! I've pushed it to emacs-26 now.

Hello Dmitry,

thanks a lot. Would you mind installing the attached whitespace fixes
as well as a follow up?

Cheers
Bastian

[-- Attachment #2: whitespace.patch --]
[-- Type: text/x-patch, Size: 1295 bytes --]

diff --git a/lisp/cedet/semantic/decorate/include.el b/lisp/cedet/semantic/decorate/include.el
index 9f1825d420..975ba34346 100644
--- a/lisp/cedet/semantic/decorate/include.el
+++ b/lisp/cedet/semantic/decorate/include.el
@@ -467,7 +467,7 @@ its contents.
 	(error "Could not location include %s"
 	       (semantic-tag-name tag)))
        ((get-file-buffer file)
-       (pop-to-buffer-same-window (get-file-buffer file)))
+        (pop-to-buffer-same-window (get-file-buffer file)))
        ((stringp file)
 	(find-file file))
        ))))
diff --git a/lisp/cedet/semantic/senator.el b/lisp/cedet/semantic/senator.el
index 70e04475ab..ea796dd19f 100644
--- a/lisp/cedet/semantic/senator.el
+++ b/lisp/cedet/semantic/senator.el
@@ -530,11 +530,11 @@ Some tags such as includes have other reference features."
        ;; A tag
        ((semantic-tag-p result)
 	(semantic-go-to-tag result)
-       (pop-to-buffer-same-window (current-buffer))
+        (pop-to-buffer-same-window (current-buffer))
 	(semantic-momentary-highlight-tag result))
        ;; Buffers
        ((bufferp result)
-       (pop-to-buffer-same-window result)
+        (pop-to-buffer-same-window result)
 	(pulse-momentary-highlight-one-line (point)))
        ;; Files
        ((and (stringp result) (file-exists-p result))

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* bug#28645: Status: 26.0.50; semantic-ia-fast-jump jumps to a random place in buffer
  2017-10-19  9:41             ` Bastian Beischer
@ 2017-10-19 10:03               ` Dmitry Gutov
  0 siblings, 0 replies; 17+ messages in thread
From: Dmitry Gutov @ 2017-10-19 10:03 UTC (permalink / raw)
  To: Bastian Beischer; +Cc: bug#28645

On 10/19/17 12:41 PM, Bastian Beischer wrote:

> thanks a lot. Would you mind installing the attached whitespace fixes
> as well as a follow up?

Also done, thanks!





^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2017-10-19 10:03 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-29 13:23 bug#28645: 26.0.50; semantic-ia-fast-jump jumps to a random place in buffer Constantine
2017-09-29 14:42 ` Dmitry Gutov
2017-09-29 14:57   ` Constantine
2017-09-29 18:18   ` martin rudalics
2017-09-30 14:19     ` Dmitry Gutov
2017-10-03  9:40 ` bug#28645: Status: " Bastian Beischer
2017-10-04  9:03   ` martin rudalics
2017-10-04 11:11     ` Bastian Beischer
2017-10-05  8:09       ` martin rudalics
2017-10-15 23:06   ` Dmitry Gutov
2017-10-16 10:21     ` Bastian Beischer
2017-10-17  8:58       ` martin rudalics
2017-10-17 10:44       ` Dmitry Gutov
2017-10-17 12:30         ` Bastian Beischer
2017-10-18 23:46           ` Dmitry Gutov
2017-10-19  9:41             ` Bastian Beischer
2017-10-19 10:03               ` Dmitry Gutov

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).