* comment in managedwin.itb
@ 2000-11-28 12:47 Tom Tromey
2000-11-28 12:54 ` Mo DeJong
2000-11-28 13:00 ` Syd Polk
0 siblings, 2 replies; 11+ messages in thread
From: Tom Tromey @ 2000-11-28 12:47 UTC (permalink / raw)
To: Insight List
I see this code in managedwin.itb:
# I don't understand this next line and no one commented it, so it's gone.
#focus -force [focus -lastfor $top]
focus $top
I think I probably wrote the commented-out code. Finding who
commented it out was annoying (most copies of the code in CVS were
made by people who obviously weren't the author), but I think it was
Jim Ingham.
Either I didn't comment this because I thought it was an idiom (I did
document it in my Tcl style guide), or because we were under a lot of
time pressure, or both.
The basic idea here is that `focus $top' sets the focus window to $top
-- but if the focus was previously on a subwindow of $top, then this
information is lost.
The idiom `focus -force [focus -lastfor $top]' causes us to force the
focus to change, but also lets us preserve the focus inside the
window.
Using -force is relatively unfriendly though. I think we were doing
that everywhere at the time because we were targeting Windows, and
that seems to be the sort of thing one does there.
As it stands now I think the comment and the existing focus command in
that proc could be removed. Comments on this?
Tom
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: comment in managedwin.itb
2000-11-28 12:47 comment in managedwin.itb Tom Tromey
@ 2000-11-28 12:54 ` Mo DeJong
2000-11-28 13:00 ` Syd Polk
1 sibling, 0 replies; 11+ messages in thread
From: Mo DeJong @ 2000-11-28 12:54 UTC (permalink / raw)
To: insight
On 28 Nov 2000, Tom Tromey wrote:
> I see this code in managedwin.itb:
>
> # I don't understand this next line and no one commented it, so it's gone.
> #focus -force [focus -lastfor $top]
>
> focus $top
If you are looking for a better way to do that, you
could use this proc. It does the refocus management
with a -force except that it also handles the case
where the focus has not yet been assigned to a
widget in the given toplevel.
proc _refocus { window } {
set toplevel [winfo toplevel $window]
set last [focus -lastfor $window]
if {$toplevel == $last} {
focus -force $window
} else {
focus -force $last
}
}
Just call it like so:
_refocus $text
Of course, I am assuming that you want to actually snap
the focus to the given widget with -force here.
Mo DeJong
Red Hat Inc
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: comment in managedwin.itb
2000-11-28 12:47 comment in managedwin.itb Tom Tromey
2000-11-28 12:54 ` Mo DeJong
@ 2000-11-28 13:00 ` Syd Polk
2000-11-28 13:08 ` Keith Seitz
2000-11-28 20:17 ` Tom Tromey
1 sibling, 2 replies; 11+ messages in thread
From: Syd Polk @ 2000-11-28 13:00 UTC (permalink / raw)
To: tromey, Insight List
I hate commented it out code. I think you should restore the command and
give a concise comment about why you are not using focus $top.
At 01:57 PM 11/28/00 -0700, Tom Tromey wrote:
>I see this code in managedwin.itb:
>
> # I don't understand this next line and no one commented it, so it's gone.
> #focus -force [focus -lastfor $top]
>
> focus $top
>
>
>I think I probably wrote the commented-out code. Finding who
>commented it out was annoying (most copies of the code in CVS were
>made by people who obviously weren't the author), but I think it was
>Jim Ingham.
>
>Either I didn't comment this because I thought it was an idiom (I did
>document it in my Tcl style guide), or because we were under a lot of
>time pressure, or both.
>
>The basic idea here is that `focus $top' sets the focus window to $top
>-- but if the focus was previously on a subwindow of $top, then this
>information is lost.
>
>The idiom `focus -force [focus -lastfor $top]' causes us to force the
>focus to change, but also lets us preserve the focus inside the
>window.
>
>Using -force is relatively unfriendly though. I think we were doing
>that everywhere at the time because we were targeting Windows, and
>that seems to be the sort of thing one does there.
>
>As it stands now I think the comment and the existing focus command in
>that proc could be removed. Comments on this?
>
>Tom
Syd Polk spolk@redhat.com
Engineering Manager +1 415 777 9810 x 241
Red Hat, Inc.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: comment in managedwin.itb
2000-11-28 13:00 ` Syd Polk
@ 2000-11-28 13:08 ` Keith Seitz
2000-11-28 13:16 ` Fernando Nasser
2000-11-28 20:17 ` Tom Tromey
1 sibling, 1 reply; 11+ messages in thread
From: Keith Seitz @ 2000-11-28 13:08 UTC (permalink / raw)
To: tromey; +Cc: Insight List
Syd Polk wrote:
>
> At 01:57 PM 11/28/00 -0700, Tom Tromey wrote:
> >I see this code in managedwin.itb:
> >
> > # I don't understand this next line and no one commented it, so it's gone.
> > #focus -force [focus -lastfor $top]
> >
> > focus $top
> >
> >
> >I think I probably wrote the commented-out code. Finding who
> >commented it out was annoying (most copies of the code in CVS were
> >made by people who obviously weren't the author), but I think it was
> >Jim Ingham.
It could have been me, too. I don't recall anymore... In any case, I
thought that this screwed up certain dialogs, TargetSelection in
particular. (Or maybe it did this just on windows??? Or when something
went awry...)
Anyway, I've said something now so it's in the archives. Maybe we'll
find out why this was done later. Perhaps there is some context in the
changelog?
Keith
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: comment in managedwin.itb
2000-11-28 13:08 ` Keith Seitz
@ 2000-11-28 13:16 ` Fernando Nasser
0 siblings, 0 replies; 11+ messages in thread
From: Fernando Nasser @ 2000-11-28 13:16 UTC (permalink / raw)
To: Keith Seitz; +Cc: tromey, Insight List
Keith Seitz wrote:
>
> Syd Polk wrote:
> >
> > At 01:57 PM 11/28/00 -0700, Tom Tromey wrote:
> > >I see this code in managedwin.itb:
> > >
> > > # I don't understand this next line and no one commented it, so it's gone.
> > > #focus -force [focus -lastfor $top]
> > >
> > > focus $top
> > >
> > >
> > >I think I probably wrote the commented-out code. Finding who
> > >commented it out was annoying (most copies of the code in CVS were
> > >made by people who obviously weren't the author), but I think it was
> > >Jim Ingham.
>
> It could have been me, too. I don't recall anymore... In any case, I
> thought that this screwed up certain dialogs, TargetSelection in
> particular. (Or maybe it did this just on windows??? Or when something
> went awry...)
>
> Anyway, I've said something now so it's in the archives. Maybe we'll
> find out why this was done later. Perhaps there is some context in the
> changelog?
>
Unfortunately the comment did not mention what was broken and where.
IMO, although it is a very boring task, we should try to research all the possible
sources before turning this on again (we would do it if the research fails
and re-learn it the hard way -- probably through a regression). Unless the
current code is causing some other annoyance, in which case we have no choice but
to try different things (it is broken one way or another).
--
Fernando Nasser
Red Hat - Toronto E-Mail: fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario M4P 2C9
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: comment in managedwin.itb
2000-11-28 13:00 ` Syd Polk
2000-11-28 13:08 ` Keith Seitz
@ 2000-11-28 20:17 ` Tom Tromey
2000-11-29 9:40 ` Fernando Nasser
1 sibling, 1 reply; 11+ messages in thread
From: Tom Tromey @ 2000-11-28 20:17 UTC (permalink / raw)
To: Syd Polk; +Cc: Insight List
Syd> I hate commented it out code. I think you should restore the
Syd> command and give a concise comment about why you are not using
Syd> focus $top.
Actually, I think we shouldn't use focus at all.
* Using focus -force is unfriendly. It steals the focus, which the
user might not want.
* Using focus is pointless since it just moves the potential focus
point in that window. In general we don't want to do that.
How about this?
2000-11-28 Tom Tromey <tromey@cygnus.com>
* managedwin.itb (ManagedWin::reveal): Don't set focus.
Tom
Index: managedwin.itb
===================================================================
RCS file: /cvs/src/src/gdb/gdbtk/library/managedwin.itb,v
retrieving revision 1.4
diff -u -r1.4 managedwin.itb
--- managedwin.itb 2000/03/29 03:18:16 1.4
+++ managedwin.itb 2000/11/29 04:16:39
@@ -40,11 +40,6 @@
set top [winfo toplevel [namespace tail $this]]
raise $top
wm deiconify $top
-
- # I don't understand this next line and no one commented it, so it's gone.
- #focus -force [focus -lastfor $top]
-
- focus $top
}
body ManagedWin::restart {} {
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: comment in managedwin.itb
2000-11-28 20:17 ` Tom Tromey
@ 2000-11-29 9:40 ` Fernando Nasser
2000-12-07 14:18 ` Fernando Nasser
0 siblings, 1 reply; 11+ messages in thread
From: Fernando Nasser @ 2000-11-29 9:40 UTC (permalink / raw)
To: tromey; +Cc: Syd Polk, Insight List
As nobody seems to be 100% sure of how this happened, I suggest you add your
comments (between -----) and comment out the focus command.
Lets see what happens and, if we are all happy with the results, we remove
the code altogether (this is how we have been handling these cases in gdb).
We may even leave your comments permanently so no one in the future try do
add a "focus" in there.
Fernando
Tom Tromey wrote:
>
> Syd> I hate commented it out code. I think you should restore the
> Syd> command and give a concise comment about why you are not using
> Syd> focus $top.
>
------
> Actually, I think we shouldn't use focus at all.
>
> * Using focus -force is unfriendly. It steals the focus, which the
> user might not want.
> * Using focus is pointless since it just moves the potential focus
> point in that window. In general we don't want to do that.
------
>
> How about this?
>
> 2000-11-28 Tom Tromey <tromey@cygnus.com>
>
> * managedwin.itb (ManagedWin::reveal): Don't set focus.
>
> Tom
>
> Index: managedwin.itb
> ===================================================================
> RCS file: /cvs/src/src/gdb/gdbtk/library/managedwin.itb,v
> retrieving revision 1.4
> diff -u -r1.4 managedwin.itb
> --- managedwin.itb 2000/03/29 03:18:16 1.4
> +++ managedwin.itb 2000/11/29 04:16:39
> @@ -40,11 +40,6 @@
> set top [winfo toplevel [namespace tail $this]]
> raise $top
> wm deiconify $top
> -
> - # I don't understand this next line and no one commented it, so it's gone.
> - #focus -force [focus -lastfor $top]
> -
> - focus $top
> }
>
> body ManagedWin::restart {} {
--
Fernando Nasser
Red Hat - Toronto E-Mail: fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario M4P 2C9
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: comment in managedwin.itb
2000-11-29 9:40 ` Fernando Nasser
@ 2000-12-07 14:18 ` Fernando Nasser
2000-12-07 14:40 ` Tom Tromey
0 siblings, 1 reply; 11+ messages in thread
From: Fernando Nasser @ 2000-12-07 14:18 UTC (permalink / raw)
To: tromey, Syd Polk, Insight List
Tom,
I guess you did not take it as approval. My only suggestion is that you add
your comments (what you say in between the ------ below) in there so that no
one goes and add a "focus" back in that place.
Or do you think having "focus" in there is so absurd that after you remove it
no one will ever think of doing that?
Regards,
Fernando
Fernando Nasser wrote:
>
> As nobody seems to be 100% sure of how this happened, I suggest you add your
> comments (between -----) and comment out the focus command.
>
> Lets see what happens and, if we are all happy with the results, we remove
> the code altogether (this is how we have been handling these cases in gdb).
> We may even leave your comments permanently so no one in the future try do
> add a "focus" in there.
>
> Fernando
>
> Tom Tromey wrote:
> >
> > Syd> I hate commented it out code. I think you should restore the
> > Syd> command and give a concise comment about why you are not using
> > Syd> focus $top.
> >
> ------
> > Actually, I think we shouldn't use focus at all.
> >
> > * Using focus -force is unfriendly. It steals the focus, which the
> > user might not want.
> > * Using focus is pointless since it just moves the potential focus
> > point in that window. In general we don't want to do that.
> ------
> >
> > How about this?
> >
> > 2000-11-28 Tom Tromey <tromey@cygnus.com>
> >
> > * managedwin.itb (ManagedWin::reveal): Don't set focus.
> >
> > Tom
> >
> > Index: managedwin.itb
> > ===================================================================
> > RCS file: /cvs/src/src/gdb/gdbtk/library/managedwin.itb,v
> > retrieving revision 1.4
> > diff -u -r1.4 managedwin.itb
> > --- managedwin.itb 2000/03/29 03:18:16 1.4
> > +++ managedwin.itb 2000/11/29 04:16:39
> > @@ -40,11 +40,6 @@
> > set top [winfo toplevel [namespace tail $this]]
> > raise $top
> > wm deiconify $top
> > -
> > - # I don't understand this next line and no one commented it, so it's gone.
> > - #focus -force [focus -lastfor $top]
> > -
> > - focus $top
> > }
> >
> > body ManagedWin::restart {} {
>
--
Fernando Nasser
Red Hat Canada Ltd. E-Mail: fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario M4P 2C9
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: comment in managedwin.itb
2000-12-07 14:18 ` Fernando Nasser
@ 2000-12-07 14:40 ` Tom Tromey
2000-12-07 15:15 ` Fernando Nasser
0 siblings, 1 reply; 11+ messages in thread
From: Tom Tromey @ 2000-12-07 14:40 UTC (permalink / raw)
To: Fernando Nasser; +Cc: Syd Polk, Insight List
Fernando> I guess you did not take it as approval. My only suggestion
Fernando> is that you add your comments (what you say in between the
Fernando> ------ below) in there so that no one goes and add a "focus"
Fernando> back in that place.
Fernando> Or do you think having "focus" in there is so absurd that
Fernando> after you remove it no one will ever think of doing that?
Nope.
I deleted the code and added this comment:
# There used to be a `focus -force' here, but using -force is
# unfriendly, so it was removed. It was then replaced with a simple
# `focus $top'. However, this has no useful effect -- it just
# resets the subwindow of $top which has the `potential' focus.
# This can actually be confusing to the user.
I'm checking this in. If you want something more in the comment, tell
me and I will add it.
Tom
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: comment in managedwin.itb
2000-12-07 14:40 ` Tom Tromey
@ 2000-12-07 15:15 ` Fernando Nasser
0 siblings, 0 replies; 11+ messages in thread
From: Fernando Nasser @ 2000-12-07 15:15 UTC (permalink / raw)
To: tromey; +Cc: Syd Polk, Insight List
Tom Tromey wrote:
>
> Fernando> I guess you did not take it as approval. My only suggestion
> Fernando> is that you add your comments (what you say in between the
> Fernando> ------ below) in there so that no one goes and add a "focus"
> Fernando> back in that place.
>
> Fernando> Or do you think having "focus" in there is so absurd that
> Fernando> after you remove it no one will ever think of doing that?
>
> Nope.
>
> I deleted the code and added this comment:
>
> # There used to be a `focus -force' here, but using -force is
> # unfriendly, so it was removed. It was then replaced with a simple
> # `focus $top'. However, this has no useful effect -- it just
> # resets the subwindow of $top which has the `potential' focus.
> # This can actually be confusing to the user.
>
> I'm checking this in. If you want something more in the comment, tell
> me and I will add it.
>
Excelent! That would prevent regressions.
Thank you.
--
Fernando Nasser
Red Hat Canada Ltd. E-Mail: fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario M4P 2C9
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: comment in managedwin.itb
[not found] <975459464.28425.ezmlm@sources.redhat.com>
@ 2000-11-28 19:19 ` Jim Ingham
0 siblings, 0 replies; 11+ messages in thread
From: Jim Ingham @ 2000-11-28 19:19 UTC (permalink / raw)
To: insight
Keith,
Actually, IIRC this comment came from Martin. I merged a whole bunch of his changes onto the main line at some point, so I ended up being the cvs annotate author of lots of stuff that was Martin's rather than mine. But I remember vaguely having some conversation with him on the phone about this, but sadly not in any detail, so I can't really shed light on why we left this out.
Sorry...
Jim
> From: Keith Seitz <kseitz@firetalk.com>
> Date: Tue Nov 28, 2000 01:07:54 PM US/Pacific
> To: tromey@cygnus.com
> Cc: Insight List <insight@sourceware.cygnus.com>
> Subject: Re: comment in managedwin.itb
>
> Syd Polk wrote:
> >
> > At 01:57 PM 11/28/00 -0700, Tom Tromey wrote:
> > >I see this code in managedwin.itb:
> > >
> > > # I don't understand this next line and no one commented it, so it'
> s gone.
> > > #focus -force [focus -lastfor $top]
> > >
> > > focus $top
> > >
> > >
> > >I think I probably wrote the commented-out code. Finding who
> > >commented it out was annoying (most copies of the code in CVS were
> > >made by people who obviously weren't the author), but I think it was
> > >Jim Ingham.
>
> It could have been me, too. I don't recall anymore... In any case, I
> thought that this screwed up certain dialogs, TargetSelection in
> particular. (Or maybe it did this just on windows??? Or when something
> went awry...)
>
> Anyway, I've said something now so it's in the archives. Maybe we'll
> find out why this was done later. Perhaps there is some context in the
> changelog?
>
> Keith
--
Jim Ingham jingham@apple.com
Developer Tools - gdb
Apple Computer
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2000-12-07 15:15 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2000-11-28 12:47 comment in managedwin.itb Tom Tromey
2000-11-28 12:54 ` Mo DeJong
2000-11-28 13:00 ` Syd Polk
2000-11-28 13:08 ` Keith Seitz
2000-11-28 13:16 ` Fernando Nasser
2000-11-28 20:17 ` Tom Tromey
2000-11-29 9:40 ` Fernando Nasser
2000-12-07 14:18 ` Fernando Nasser
2000-12-07 14:40 ` Tom Tromey
2000-12-07 15:15 ` Fernando Nasser
[not found] <975459464.28425.ezmlm@sources.redhat.com>
2000-11-28 19:19 ` Jim Ingham
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).