unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* [PATCH] find-file-noselect-1
@ 2005-02-10 20:36 Nick Roberts
  2005-02-11  0:19 ` Miles Bader
  2005-02-11  3:19 ` Stefan Monnier
  0 siblings, 2 replies; 22+ messages in thread
From: Nick Roberts @ 2005-02-10 20:36 UTC (permalink / raw)



I would like to change find-file-noselect-1, which is called by find-file.  It
would mean that if you visit a file that is part of the source code of the
current GDB session, then you can set a breakpoint by clicking in the
fringe. Pretty good huh! It could also mean that I've broken find-file, just
before the release. Not so good. Thats why I am posting it here first.

Nick


*** /home/nick/emacs/lisp/files.el.~1.745.~	2005-02-10 08:22:48.000000000 +1300
--- /home/nick/emacs/lisp/files.el	2005-02-11 09:06:39.000000000 +1300
***************
*** 1511,1516 ****
--- 1511,1525 ----
  	    (make-local-variable 'find-file-literally)
  	    (setq find-file-literally t))
  	(after-find-file error (not nowarn)))
+       (if (and (boundp 'gud-comint-buffer)
+ 	       (buffer-name gud-comint-buffer)
+ 	       (with-current-buffer gud-comint-buffer
+ 		 (eq gud-minor-mode 'gdba)))
+ 	  (progn
+ 	    (gdb-enqueue-input
+ 	     (list (concat "list " (file-name-nondirectory buffer-file-name)
+ 			   ":1\n")
+ 		   `(lambda () (gdb-set-gud-minor-mode ,buffer-file-name))))))
        (current-buffer))))
  \f
  (defun insert-file-contents-literally (filename &optional visit beg end replace)


*** /home/nick/emacs/lisp/progmodes/gdb-ui.el.~1.47.~	2005-02-10 08:22:54.000000000 +1300
--- /home/nick/emacs/lisp/progmodes/gdb-ui.el	2005-02-11 09:07:10.000000000 +1300
***************
*** 2085,2090 ****
--- 2085,2100 ----
        (goto-line (string-to-number line))
        (gdb-put-breakpoint-icon (eq flag ?y) bptno))))
  
+ (defun gdb-set-gud-minor-mode (file)
+   "Set gud-minor-mode from find-file if appropriate."
+   (goto-char (point-min))
+   (unless (search-forward "No source file named " nil t)
+       (with-current-buffer
+ 	  (find-file-noselect file)
+ 	(save-current-buffer
+ 	  (set (make-local-variable 'gud-minor-mode) 'gdba)
+ 	  (set (make-local-variable 'tool-bar-map) gud-tool-bar-map)))))
+ 
  ;;from put-image
  (defun gdb-put-string (putstring pos &optional dprop)
    "Put string PUTSTRING in front of POS in the current buffer.

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

* Re: [PATCH] find-file-noselect-1
  2005-02-10 20:36 [PATCH] find-file-noselect-1 Nick Roberts
@ 2005-02-11  0:19 ` Miles Bader
  2005-02-11  2:49   ` Nick Roberts
  2005-02-11  3:19 ` Stefan Monnier
  1 sibling, 1 reply; 22+ messages in thread
From: Miles Bader @ 2005-02-11  0:19 UTC (permalink / raw)
  Cc: emacs-devel

Is this not something that could be served by a hook?  Or _should_ be
served by a hook?  After all, if gdb-mode wants to do this sort of
thing, mightn't others?

Sticking gdb-specific code in the basic find-file machinery seems
pretty ugly -- and if it's _really_ needed, at least make it a single
function call rather than a big glob of inline gdb-specific code!

Thanks,

-Miles

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

* Re: [PATCH] find-file-noselect-1
  2005-02-11  0:19 ` Miles Bader
@ 2005-02-11  2:49   ` Nick Roberts
  2005-02-11  3:03     ` Nick Roberts
  2005-02-11  3:16     ` Stefan Monnier
  0 siblings, 2 replies; 22+ messages in thread
From: Nick Roberts @ 2005-02-11  2:49 UTC (permalink / raw)
  Cc: emacs-devel

 > Is this not something that could be served by a hook?  Or _should_ be
 > served by a hook?

Yes, that's good feedback. vc adds vc-find-file-hook to find-file-hook
in vc-hooks.el. However, that file is built into Emacs through loadup.
Where could I add such a hook, so that it would always be included?

Nick

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

* Re: [PATCH] find-file-noselect-1
  2005-02-11  2:49   ` Nick Roberts
@ 2005-02-11  3:03     ` Nick Roberts
  2005-02-12  8:38       ` Richard Stallman
  2005-02-11  3:16     ` Stefan Monnier
  1 sibling, 1 reply; 22+ messages in thread
From: Nick Roberts @ 2005-02-11  3:03 UTC (permalink / raw)



 > Where could I add such a hook, so that it would always be included?

Answering my own question: it can be added in gdb-ui.el. The following patch
is only for gdb-ui.el, but it adds a function to find-file-hook. I guess it
could still break find-file, if its not right.

Is it OK to install this?

Nick

*** /home/nick/emacs/lisp/progmodes/gdb-ui.el.~1.47.~	2005-02-10 08:22:54.000000000 +1300
--- /home/nick/emacs/lisp/progmodes/gdb-ui.el	2005-02-11 15:53:01.000000000 +1300
***************
*** 2085,2090 ****
--- 2085,2113 ----
        (goto-line (string-to-number line))
        (gdb-put-breakpoint-icon (eq flag ?y) bptno))))
  
+ (add-hook 'find-file-hook 'gdb-find-file-hook)
+ 
+ (defun gdb-find-file-hook ()
+   (if (and (boundp 'gud-comint-buffer)
+ 	   (buffer-name gud-comint-buffer)
+ 	   (with-current-buffer gud-comint-buffer
+ 	     (eq gud-minor-mode 'gdba)))
+       (progn
+ 	(gdb-enqueue-input
+ 	 (list (concat "list " (file-name-nondirectory buffer-file-name)
+ 		       ":1\n")
+ 	       `(lambda () (gdb-set-gud-minor-mode ,buffer-file-name)))))))
+ 
+ (defun gdb-set-gud-minor-mode (file)
+   "Set gud-minor-mode from find-file if appropriate."
+   (goto-char (point-min))
+   (unless (search-forward "No source file named " nil t)
+       (with-current-buffer
+ 	  (find-file-noselect file)
+ 	(save-current-buffer
+ 	  (set (make-local-variable 'gud-minor-mode) 'gdba)
+ 	  (set (make-local-variable 'tool-bar-map) gud-tool-bar-map)))))
+ 
  ;;from put-image
  (defun gdb-put-string (putstring pos &optional dprop)
    "Put string PUTSTRING in front of POS in the current buffer.

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

* Re: [PATCH] find-file-noselect-1
  2005-02-11  2:49   ` Nick Roberts
  2005-02-11  3:03     ` Nick Roberts
@ 2005-02-11  3:16     ` Stefan Monnier
  1 sibling, 0 replies; 22+ messages in thread
From: Stefan Monnier @ 2005-02-11  3:16 UTC (permalink / raw)
  Cc: snogglethorpe, emacs-devel, miles

>> Is this not something that could be served by a hook?  Or _should_ be
>> served by a hook?

> Yes, that's good feedback. vc adds vc-find-file-hook to find-file-hook
> in vc-hooks.el. However, that file is built into Emacs through loadup.
> Where could I add such a hook, so that it would always be included?

Why would you always need it?  I mean it can only needed if gdba.el is
loaded, right?
So what's wrong with adding

  (add-hook 'find-file-hook 'gdb-find-file-hook)

at the top-level of gdba.el ?


        Stefan

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

* Re: [PATCH] find-file-noselect-1
  2005-02-10 20:36 [PATCH] find-file-noselect-1 Nick Roberts
  2005-02-11  0:19 ` Miles Bader
@ 2005-02-11  3:19 ` Stefan Monnier
  2005-02-11  8:08   ` Nick Roberts
  1 sibling, 1 reply; 22+ messages in thread
From: Stefan Monnier @ 2005-02-11  3:19 UTC (permalink / raw)
  Cc: emacs-devel

> I would like to change find-file-noselect-1, which is called by find-file.
> It would mean that if you visit a file that is part of the source code of
> the current GDB session, then you can set a breakpoint by clicking in the
> fringe. Pretty good huh! It could also mean that I've broken find-file,
> just before the release.  Not so good.  Thats why I am posting it
> here first.

BTW, I firmly vote against any such change (even if done via
find-file-hook): gdba.el should be trying to be more robust (in the face of
unexpected or missing output from gdb), not more featureful.

Otherwise, we run the risk that we'll need to make gdb's default
"gdb --fullname" rather than "gdb --annotate=3" if too many bug-reports come
in and we can't fix them in time.


        Stefan

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

* Re: [PATCH] find-file-noselect-1
  2005-02-11  3:19 ` Stefan Monnier
@ 2005-02-11  8:08   ` Nick Roberts
  2005-02-11  8:51     ` Kim F. Storm
  0 siblings, 1 reply; 22+ messages in thread
From: Nick Roberts @ 2005-02-11  8:08 UTC (permalink / raw)
  Cc: emacs-devel

 > BTW, I firmly vote against any such change (even if done via
 > find-file-hook): gdba.el should be trying to be more robust (in the face of
 > unexpected or missing output from gdb), not more featureful.

Currently, in a debug session with gdb-ui, the first breakpoint has to be set
either with a global binding e.g C-x SPC or through the GUD buffer. No other
debugger, that I know of, puts such an unintuitive constraint on the user.
This patch makes the fringe available for setting breakpoints at the start
of the session. I wouldn't call that a adding new feature, but rather making
an existing one more sound.
 
 > Otherwise, we run the risk that we'll need to make gdb's default
 > "gdb --fullname" rather than "gdb --annotate=3" if too many bug-reports come
 > in and we can't fix them in time.

Not many bug reports have come in, probably because not many people have usd
gdb-ui, but thats a separate risk to the one of using find-file-hook. Anyway,
lets wait until too many bug-reports do come in before talking about changing
the default.

If a user doesn't use GDB, then there's no risk because the hook doesn't get
called. If a user does use GDB, and finds a problem with gdb-ui, then he's
probably competent enough to set the default to "gdb --fullname" where the
hook does nothing.

GDB isn't just another application, its part of the GNU Project and, as a
debugger, intimately linked with the editor (I think it even used to be
distributed on the same tape as Emacs). I think its important that these two
programs should be made to work well with each other and that, in the
long term, the benfeit is worth the risk.

Nick

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

* Re: [PATCH] find-file-noselect-1
  2005-02-11  8:08   ` Nick Roberts
@ 2005-02-11  8:51     ` Kim F. Storm
  2005-02-11  9:53       ` Nick Roberts
  2005-02-11 10:05       ` Miles Bader
  0 siblings, 2 replies; 22+ messages in thread
From: Kim F. Storm @ 2005-02-11  8:51 UTC (permalink / raw)
  Cc: Stefan Monnier, emacs-devel

Nick Roberts <nickrob@snap.net.nz> writes:

>  > BTW, I firmly vote against any such change (even if done via
>  > find-file-hook): gdba.el should be trying to be more robust (in the face of
>  > unexpected or missing output from gdb), not more featureful.
>
> Currently, in a debug session with gdb-ui, the first breakpoint has to be set
> either with a global binding e.g C-x SPC or through the GUD buffer. No other
> debugger, that I know of, puts such an unintuitive constraint on the user.
> This patch makes the fringe available for setting breakpoints at the start
> of the session. I wouldn't call that a adding new feature, but rather making
> an existing one more sound.

I agree with you that we need to have this feature.

But can't gdb-ui just bind [left-fringe mouse-1] to a gdb-ui specific
command in global-map?  That command could check if the buffer is
a source file  and set breakpoints accordingly (if gdb is running
and in a state that allows it to set a breakpoint), and just
do mouse-set-point otherwise.

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

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

* Re: [PATCH] find-file-noselect-1
  2005-02-11  8:51     ` Kim F. Storm
@ 2005-02-11  9:53       ` Nick Roberts
  2005-02-11 10:30         ` Kim F. Storm
  2005-02-11 10:05       ` Miles Bader
  1 sibling, 1 reply; 22+ messages in thread
From: Nick Roberts @ 2005-02-11  9:53 UTC (permalink / raw)
  Cc: Stefan Monnier, emacs-devel

 > I agree with you that we need to have this feature.
 > 
 > But can't gdb-ui just bind [left-fringe mouse-1] to a gdb-ui specific
 > command in global-map?  That command could check if the buffer is
 > a source file  and set breakpoints accordingly (if gdb is running
 > and in a state that allows it to set a breakpoint), and just
 > do mouse-set-point otherwise.

That would be fine with me, but what if later someone else wants to use
[left-fringe mouse-1] for another purpose?

Nick

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

* Re: [PATCH] find-file-noselect-1
  2005-02-11  8:51     ` Kim F. Storm
  2005-02-11  9:53       ` Nick Roberts
@ 2005-02-11 10:05       ` Miles Bader
  2005-02-11 10:29         ` Kim F. Storm
  1 sibling, 1 reply; 22+ messages in thread
From: Miles Bader @ 2005-02-11 10:05 UTC (permalink / raw)
  Cc: Nick Roberts, Stefan Monnier, emacs-devel

On Fri, 11 Feb 2005 09:51:59 +0100, Kim F. Storm <storm@cua.dk> wrote:
> But can't gdb-ui just bind [left-fringe mouse-1] to a gdb-ui specific
> command in global-map?  That command could check if the buffer is
> a source file  and set breakpoints accordingly (if gdb is running
> and in a state that allows it to set a breakpoint), and just
> do mouse-set-point otherwise.

Nick's method -- as I understand it -- seems cleaner:  when a new file
is opened, see if gdb wants to deal with it (and if so, do
...whatever... magic).

-Miles
-- 
Do not taunt Happy Fun Ball.

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

* Re: [PATCH] find-file-noselect-1
  2005-02-11 10:05       ` Miles Bader
@ 2005-02-11 10:29         ` Kim F. Storm
  2005-02-11 10:44           ` Nick Roberts
  0 siblings, 1 reply; 22+ messages in thread
From: Kim F. Storm @ 2005-02-11 10:29 UTC (permalink / raw)
  Cc: Nick Roberts, emacs-devel, Stefan Monnier, miles

Miles Bader <snogglethorpe@gmail.com> writes:

> On Fri, 11 Feb 2005 09:51:59 +0100, Kim F. Storm <storm@cua.dk> wrote:
>> But can't gdb-ui just bind [left-fringe mouse-1] to a gdb-ui specific
>> command in global-map?  That command could check if the buffer is
>> a source file  and set breakpoints accordingly (if gdb is running
>> and in a state that allows it to set a breakpoint), and just
>> do mouse-set-point otherwise.
>
> Nick's method -- as I understand it -- seems cleaner:  when a new file
> is opened, see if gdb wants to deal with it (and if so, do
> ...whatever... magic).

But what if I have N files opened already and then load/start gdb -- 
are those files already opened "gdb enabled" using Nick's method?

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

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

* Re: [PATCH] find-file-noselect-1
  2005-02-11  9:53       ` Nick Roberts
@ 2005-02-11 10:30         ` Kim F. Storm
  0 siblings, 0 replies; 22+ messages in thread
From: Kim F. Storm @ 2005-02-11 10:30 UTC (permalink / raw)
  Cc: Stefan Monnier, emacs-devel

Nick Roberts <nickrob@snap.net.nz> writes:

>  > I agree with you that we need to have this feature.
>  > 
>  > But can't gdb-ui just bind [left-fringe mouse-1] to a gdb-ui specific
>  > command in global-map?  That command could check if the buffer is
>  > a source file  and set breakpoints accordingly (if gdb is running
>  > and in a state that allows it to set a breakpoint), and just
>  > do mouse-set-point otherwise.
>
> That would be fine with me, but what if later someone else wants to use
> [left-fringe mouse-1] for another purpose?

You got there first :-)

Other modes can bind it in their mode specific local keymap or whatever...

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

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

* Re: [PATCH] find-file-noselect-1
  2005-02-11 10:29         ` Kim F. Storm
@ 2005-02-11 10:44           ` Nick Roberts
  2005-02-11 13:05             ` Kim F. Storm
  0 siblings, 1 reply; 22+ messages in thread
From: Nick Roberts @ 2005-02-11 10:44 UTC (permalink / raw)
  Cc: snogglethorpe, emacs-devel, Stefan Monnier, miles

 > But what if I have N files opened already and then load/start gdb -- 
 > are those files already opened "gdb enabled" using Nick's method?

They aren't in the patch I presented but it would be quite easy to adapt the
hook to do that. It would just need to cycle through all the buffers, find out
which are associated with a file, test if the file is part of the source code
of the current GDB session (as in the patch), and enable for GDB if
appropriate.

Nick

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

* Re: [PATCH] find-file-noselect-1
  2005-02-11 10:44           ` Nick Roberts
@ 2005-02-11 13:05             ` Kim F. Storm
  0 siblings, 0 replies; 22+ messages in thread
From: Kim F. Storm @ 2005-02-11 13:05 UTC (permalink / raw)
  Cc: snogglethorpe, emacs-devel, Stefan Monnier, miles

Nick Roberts <nickrob@snap.net.nz> writes:

>  > But what if I have N files opened already and then load/start gdb -- 
>  > are those files already opened "gdb enabled" using Nick's method?
>
> They aren't in the patch I presented but it would be quite easy to adapt the
> hook to do that. 

In the find-file-hook function?  It should be sufficient to do this when
loading gdb-ui ?

>                  It would just need to cycle through all the buffers, find out
> which are associated with a file, test if the file is part of the source code
> of the current GDB session (as in the patch), and enable for GDB if
> appropriate.

That would be ok then.  In that case, your approach is cleaner.

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

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

* Re: [PATCH] find-file-noselect-1
  2005-02-11  3:03     ` Nick Roberts
@ 2005-02-12  8:38       ` Richard Stallman
  2005-02-12 10:30         ` Nick Roberts
  0 siblings, 1 reply; 22+ messages in thread
From: Richard Stallman @ 2005-02-12  8:38 UTC (permalink / raw)
  Cc: emacs-devel

    Answering my own question: it can be added in gdb-ui.el. The following patch
    is only for gdb-ui.el, but it adds a function to find-file-hook. I guess it
    could still break find-file, if its not right.

Bugs are always a risk--but is there any reason to think bugs are likely
in this patch?

I suggest you change the progn to condition-case so as to catch any
errors that happen in gdb-enqueue-input.  And if it does ever catch an
error, it could set gud-comint-buffer to nil so it won't do anything
on future calls to find-file.  That should make it safe.

    + (defun gdb-set-gud-minor-mode (file)
    +   "Set gud-minor-mode from find-file if appropriate."
    +   (goto-char (point-min))
    +   (unless (search-forward "No source file named " nil t)
    +       (with-current-buffer
    + 	  (find-file-noselect file)

Why call find-file-noselect there?  If this is meant to operate on the
file that was just visited, it already has a buffer, and it is the
current buffer when gdb-find-file-hook runs.  Why not just use
that buffer?

+ 	(save-current-buffer

What's the reason for that?  The contents don't change buffers.

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

* Re: [PATCH] find-file-noselect-1
  2005-02-12  8:38       ` Richard Stallman
@ 2005-02-12 10:30         ` Nick Roberts
  2005-02-12 16:50           ` Stefan Monnier
  2005-02-13 12:38           ` Richard Stallman
  0 siblings, 2 replies; 22+ messages in thread
From: Nick Roberts @ 2005-02-12 10:30 UTC (permalink / raw)
  Cc: emacs-devel

 > I suggest you change the progn to condition-case so as to catch any
 > errors that happen in gdb-enqueue-input.  And if it does ever catch an
 > error, it could set gud-comint-buffer to nil so it won't do anything
 > on future calls to find-file.  That should make it safe.

(defun gdb-find-file-hook ()
  (if (and (boundp 'gud-comint-buffer)
	   ;; in case gud or gdb-ui is just loaded.
	   gud-comint-buffer
	   (buffer-name gud-comint-buffer)
	   (with-current-buffer gud-comint-buffer
	     (eq gud-minor-mode 'gdba)))
      (condition-case nil
	(gdb-enqueue-input
	 (list (concat "list " (file-name-nondirectory buffer-file-name)
		       ":1\n")
	       `(lambda () (gdb-set-gud-minor-mode ,buffer-file-name)))))
    (error nil)))

Is this what you mean? I don't think that gud-comint-buffer should be set to
nil because that would prevent the GDB session from recovering.

 > 
 >     + (defun gdb-set-gud-minor-mode (file)
 >     +   "Set gud-minor-mode from find-file if appropriate."
 >     +   (goto-char (point-min))
 >     +   (unless (search-forward "No source file named " nil t)
 >     +       (with-current-buffer
 >     + 	  (find-file-noselect file)
 > 
 > Why call find-file-noselect there?  If this is meant to operate on the
 > file that was just visited, it already has a buffer, and it is the
 > current buffer when gdb-find-file-hook runs.  Why not just use
 > that buffer?

That might have been true but I'm now using this function to address Kim's
point about enabling gud-minor-mode for existing buffers.

 > + 	(save-current-buffer
 > 
 > What's the reason for that?  The contents don't change buffers.

This was just a legacy from earlier code. I've removed it.

Since the changes are all in gdb-ui.el, I feel more comfortable installing
them. However, having asked for people's opinion, I'll wait for some approval
before doing this.

Nick

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

* Re: [PATCH] find-file-noselect-1
  2005-02-12 10:30         ` Nick Roberts
@ 2005-02-12 16:50           ` Stefan Monnier
  2005-02-13  5:31             ` Nick Roberts
  2005-02-13 12:38           ` Richard Stallman
  1 sibling, 1 reply; 22+ messages in thread
From: Stefan Monnier @ 2005-02-12 16:50 UTC (permalink / raw)
  Cc: rms, emacs-devel

>> + (defun gdb-set-gud-minor-mode (file)
>> +   "Set gud-minor-mode from find-file if appropriate."
>> +   (goto-char (point-min))
>> +   (unless (search-forward "No source file named " nil t)
>> +       (with-current-buffer
>> + 	  (find-file-noselect file)
>> 
>> Why call find-file-noselect there?  If this is meant to operate on the
>> file that was just visited, it already has a buffer, and it is the
>> current buffer when gdb-find-file-hook runs.  Why not just use
>> that buffer?

> That might have been true but I'm now using this function to address Kim's
> point about enabling gud-minor-mode for existing buffers.

I don't understand this explanation.  In the case where you're enabling
gud-minor-mode in existing buffers, the buffers also already exist so you
shouldn't call find-file-noselect (which may cause new files to be visited).
Maybe you want something like find-buffer-visiting, but even that sounds
doubtful because it seems that you always know the buffer before you even
know the file name.


        Stefan

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

* Re: [PATCH] find-file-noselect-1
  2005-02-12 16:50           ` Stefan Monnier
@ 2005-02-13  5:31             ` Nick Roberts
  2005-02-13 16:14               ` Stefan Monnier
  0 siblings, 1 reply; 22+ messages in thread
From: Nick Roberts @ 2005-02-13  5:31 UTC (permalink / raw)
  Cc: rms, emacs-devel

>
>
>>>Why call find-file-noselect there?  If this is meant to operate on the
>>>file that was just visited, it already has a buffer, and it is the
>>>current buffer when gdb-find-file-hook runs.  Why not just use
>>>that buffer?
>>>      
>>>
>
>  
>
>>That might have been true but I'm now using this function to address Kim's
>>point about enabling gud-minor-mode for existing buffers.
>>    
>>
>
>I don't understand this explanation.  In the case where you're enabling
>gud-minor-mode in existing buffers, the buffers also already exist so you
>shouldn't call find-file-noselect (which may cause new files to be visited).
>Maybe you want something like find-buffer-visiting, but even that sounds
>doubtful because it seems that you always know the buffer before you even
>know the file name
>

I am just using find-file-noselect to recover the name of the buffer. I 
have a buffer
and a file associated with it. Generally, I don't see the big deal 
whether I pass the
buffer-name or filename across the functions. However since the code is 
part of
find-file-hook, I'll pass the buffer-name to avoid the possibility of 
some kind of
recursion.

Nick

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

* Re: [PATCH] find-file-noselect-1
  2005-02-12 10:30         ` Nick Roberts
  2005-02-12 16:50           ` Stefan Monnier
@ 2005-02-13 12:38           ` Richard Stallman
  2005-02-13 20:49             ` Nick Roberts
  1 sibling, 1 reply; 22+ messages in thread
From: Richard Stallman @ 2005-02-13 12:38 UTC (permalink / raw)
  Cc: emacs-devel

    Is this what you mean?

More or less.

			   I don't think that gud-comint-buffer should be set to
    nil because that would prevent the GDB session from recovering.

Perhaps you should add a new variable, which you can set if you catch
an error, to disable this hook from really doing anything once it has
had an error.

     > Why call find-file-noselect there?  If this is meant to operate on the
     > file that was just visited, it already has a buffer, and it is the
     > current buffer when gdb-find-file-hook runs.  Why not just use
     > that buffer?

    That might have been true but I'm now using this function to address Kim's
    point about enabling gud-minor-mode for existing buffers.

It sounds like you are in all cases trying to find an existing buffer.
So instead of using find-file-noselect, you could use
find-buffer-visiting.

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

* Re: [PATCH] find-file-noselect-1
  2005-02-13  5:31             ` Nick Roberts
@ 2005-02-13 16:14               ` Stefan Monnier
  0 siblings, 0 replies; 22+ messages in thread
From: Stefan Monnier @ 2005-02-13 16:14 UTC (permalink / raw)
  Cc: rms, emacs-devel

> I am just using find-file-noselect to recover the name of the buffer.
> I have a buffer and a file associated with it.  Generally, I don't see the
> big deal whether I pass the buffer-name or filename across the functions.

When you know that the file necessarily corresponds to a buffer and you want
to do something to the buffer rather than to the file, then you really
should pass the buffer, not the file name.

> However since the code is part of find-file-hook, I'll pass the
> buffer-name to avoid the possibility of some kind of recursion.

Better pass the *buffer* rather than the *buffer name*.


        Stefan

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

* Re: [PATCH] find-file-noselect-1
  2005-02-13 12:38           ` Richard Stallman
@ 2005-02-13 20:49             ` Nick Roberts
  2005-02-15  6:15               ` Richard Stallman
  0 siblings, 1 reply; 22+ messages in thread
From: Nick Roberts @ 2005-02-13 20:49 UTC (permalink / raw)
  Cc: emacs-devel

 > 			   I don't think that gud-comint-buffer should be set
 >     to nil because that would prevent the GDB session from recovering.
 > 
 > Perhaps you should add a new variable, which you can set if you catch
 > an error, to disable this hook from really doing anything once it has
 > had an error.

I have added a variable called gdb-find-file-unhook. It gets re-initialised
at the start of every gdb session because the user might conceivably use
different versions of gdb. The most likely cause of an error that I can see
is gdb giving unexpected output eg GDB developers are internationalising
the CLI output (Note: MI output should not change, however).

 >      > Why call find-file-noselect there?  If this is meant to operate on
 >      > the file that was just visited, it already has a buffer, and it is
 >      > the current buffer when gdb-find-file-hook runs.  Why not just use
 >      > that buffer?
 > 
 >     That might have been true but I'm now using this function to address
 >     Kim's point about enabling gud-minor-mode for existing buffers.
 > 
 > It sounds like you are in all cases trying to find an existing buffer.
 > So instead of using find-file-noselect, you could use
 > find-buffer-visiting.

I've not used find-buffer-visiting but I have followed Stefan's advice about
using the buffer instead of the filename. The patch is attached below. It
includes a few small unrelated changes now. Probably the best test is for it
to get some use.

Shall I install it?

Nick



*** /home/nick/emacs/lisp/progmodes/gdb-ui.el.~1.47.~	2005-02-10 08:22:54.000000000 +1300
--- /home/nick/emacs/lisp/progmodes/gdb-ui.el	2005-02-14 09:31:05.000000000 +1300
***************
*** 79,84 ****
--- 79,86 ----
  (defvar gdb-overlay-arrow-position nil)
  (defvar gdb-server-prefix nil)
  (defvar gdb-flush-pending-output nil)
+ (defvar gdb-location-list nil "List of directories for source files.")
+ (defvar gdb-find-file-unhook nil)
  
  (defvar gdb-buffer-type nil
    "One of the symbols bound in `gdb-buffer-rules'.")
***************
*** 191,196 ****
--- 193,227 ----
    :group 'gud
    :version "22.1")
  
+ (defun gdb-set-gud-minor-mode (buffer)
+   "Set gud-minor-mode from find-file if appropriate."
+   (goto-char (point-min))
+   (unless (search-forward "No source file named " nil t)
+     (condition-case nil
+ 	(gdb-enqueue-input
+ 	 (list (concat gdb-server-prefix "info source\n")
+ 	       `(lambda () (gdb-set-gud-minor-mode-1 ,buffer))))
+       (error (setq gdb-find-file-unhook t)))))
+ 
+ (defun gdb-set-gud-minor-mode-1 (buffer)
+   (goto-char (point-min))
+   (if (and (search-forward "Located in " nil t)
+ 	   (looking-at "\\S-*")
+ 	   (string-equal (buffer-file-name buffer)
+ 			 (match-string 0)))
+       (with-current-buffer buffer
+ 	(set (make-local-variable 'gud-minor-mode) 'gdba)
+ 	(set (make-local-variable 'tool-bar-map) gud-tool-bar-map))))
+ 
+ (defun gdb-set-gud-minor-mode-existing-buffers ()
+   (dolist (buffer (buffer-list))
+     (let ((file (buffer-file-name buffer)))
+       (if file
+ 	(progn
+ 	  (gdb-enqueue-input
+ 	   (list (concat "list " (file-name-nondirectory file) ":1\n")
+ 		 `(lambda () (gdb-set-gud-minor-mode ,buffer)))))))))
+ 
  (defun gdb-ann3 ()
    (setq gdb-debug-log nil)
    (set (make-local-variable 'gud-minor-mode) 'gdba)
***************
*** 249,254 ****
--- 280,286 ----
    (setq gdb-server-prefix "server ")
    (setq gdb-flush-pending-output nil)
    (setq gdb-location-list nil)
+   (setq gdb-find-file-unhook nil)
    ;;
    (setq gdb-buffer-type 'gdba)
    ;;
***************
*** 263,268 ****
--- 295,301 ----
    (gdb-enqueue-input (list "server list MAIN__\n" 'ignore))   ; Fortran program
    (gdb-enqueue-input (list "server info source\n" 'gdb-source-info))
    ;;
+   (gdb-set-gud-minor-mode-existing-buffers)
    (run-hooks 'gdba-mode-hook))
  
  (defcustom gdb-use-colon-colon-notation nil
***************
*** 1048,1055 ****
    ;; buffer specific functions
    gdb-info-breakpoints-custom)
  
- (defvar gdb-location-list nil "List of directories for source files.")
- 
  (defconst breakpoint-xpm-data
    "/* XPM */
  static char *magick[] = {
--- 1081,1086 ----
***************
*** 1159,1171 ****
  			   (setq file (cdr (assoc bptno gdb-location-list))))
  			(unless (string-equal file "File not found")
  			  (if file
! 			      (with-current-buffer
! 				  (find-file-noselect file)
! 				(save-current-buffer
! 				  (set (make-local-variable 'gud-minor-mode)
  				     'gdba)
! 				  (set (make-local-variable 'tool-bar-map)
! 				       gud-tool-bar-map))
  				;; only want one breakpoint icon at each location
  				(save-excursion
  				  (goto-line (string-to-number line))
--- 1190,1200 ----
  			   (setq file (cdr (assoc bptno gdb-location-list))))
  			(unless (string-equal file "File not found")
  			  (if file
! 			      (with-current-buffer (find-file-noselect file)
! 				(set (make-local-variable 'gud-minor-mode)
  				     'gdba)
! 				(set (make-local-variable 'tool-bar-map)
! 				     gud-tool-bar-map)
  				;; only want one breakpoint icon at each location
  				(save-excursion
  				  (goto-line (string-to-number line))
***************
*** 2054,2068 ****
    "Find the source file where the program starts and displays it with related
  buffers."
    (goto-char (point-min))
!   (if (search-forward "Located in " nil t)
!       (if (looking-at "\\S-*")
! 	  (setq gdb-main-file (match-string 0))))
   (if gdb-many-windows
        (gdb-setup-windows)
     (gdb-get-create-buffer 'gdb-breakpoints-buffer)
!     (if gdb-show-main
!       (let ((pop-up-windows t))
! 	(display-buffer (gud-find-file gdb-main-file))))))
  
  (defun gdb-get-location (bptno line flag)
    "Find the directory containing the relevant source file.
--- 2083,2097 ----
    "Find the source file where the program starts and displays it with related
  buffers."
    (goto-char (point-min))
!   (if (and (search-forward "Located in " nil t)
! 	   (looking-at "\\S-*"))
!       (setq gdb-main-file (match-string 0)))
   (if gdb-many-windows
        (gdb-setup-windows)
     (gdb-get-create-buffer 'gdb-breakpoints-buffer)
!    (if gdb-show-main
!        (let ((pop-up-windows t))
! 	 (display-buffer (gud-find-file gdb-main-file))))))
  
  (defun gdb-get-location (bptno line flag)
    "Find the directory containing the relevant source file.
***************
*** 2085,2090 ****
--- 2114,2135 ----
        (goto-line (string-to-number line))
        (gdb-put-breakpoint-icon (eq flag ?y) bptno))))
  
+ (add-hook 'find-file-hook 'gdb-find-file-hook)
+ 
+ (defun gdb-find-file-hook ()
+   (if (and (not gdb-find-file-unhook)
+ 	   ;; in case gud or gdb-ui is just loaded
+ 	   gud-comint-buffer
+ 	   (buffer-name gud-comint-buffer)
+ 	   (with-current-buffer gud-comint-buffer
+ 	     (eq gud-minor-mode 'gdba)))
+       (condition-case nil
+ 	(gdb-enqueue-input
+ 	 (list (concat "list " (file-name-nondirectory buffer-file-name)
+ 		       ":1\n")
+ 	       `(lambda () (gdb-set-gud-minor-mode ,(current-buffer)))))
+ 	(error nil))))
+ 
  ;;from put-image
  (defun gdb-put-string (putstring pos &optional dprop)
    "Put string PUTSTRING in front of POS in the current buffer.

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

* Re: [PATCH] find-file-noselect-1
  2005-02-13 20:49             ` Nick Roberts
@ 2005-02-15  6:15               ` Richard Stallman
  0 siblings, 0 replies; 22+ messages in thread
From: Richard Stallman @ 2005-02-15  6:15 UTC (permalink / raw)
  Cc: emacs-devel

It looks good to me, except that an error in gdb-find-file-hook
should also set gdb-find-file-unhook.

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

end of thread, other threads:[~2005-02-15  6:15 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-02-10 20:36 [PATCH] find-file-noselect-1 Nick Roberts
2005-02-11  0:19 ` Miles Bader
2005-02-11  2:49   ` Nick Roberts
2005-02-11  3:03     ` Nick Roberts
2005-02-12  8:38       ` Richard Stallman
2005-02-12 10:30         ` Nick Roberts
2005-02-12 16:50           ` Stefan Monnier
2005-02-13  5:31             ` Nick Roberts
2005-02-13 16:14               ` Stefan Monnier
2005-02-13 12:38           ` Richard Stallman
2005-02-13 20:49             ` Nick Roberts
2005-02-15  6:15               ` Richard Stallman
2005-02-11  3:16     ` Stefan Monnier
2005-02-11  3:19 ` Stefan Monnier
2005-02-11  8:08   ` Nick Roberts
2005-02-11  8:51     ` Kim F. Storm
2005-02-11  9:53       ` Nick Roberts
2005-02-11 10:30         ` Kim F. Storm
2005-02-11 10:05       ` Miles Bader
2005-02-11 10:29         ` Kim F. Storm
2005-02-11 10:44           ` Nick Roberts
2005-02-11 13:05             ` Kim F. Storm

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).