Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Daniel Jacobowitz <drow@false.org>
To: Nick Roberts <nickrob@snap.net.nz>
Cc: gdb-patches@sources.redhat.com
Subject: Re: [PATCH] PR mi/2077 "set edit off" breaks MI
Date: Fri, 17 Nov 2006 21:35:00 -0000	[thread overview]
Message-ID: <20061117213449.GC13961@nevyn.them.org> (raw)
In-Reply-To: <17719.15772.325330.48627@kahikatea.snap.net.nz>

On Thu, Oct 19, 2006 at 09:55:56PM +1300, Nick Roberts wrote:
> 
> This patch is taken out of the async patch.  It's probably a good idea to
> commit the different parts separately anyway.

Definitely.

> Apple switch interpreters which fixes this bug.  Jim Ingham can probably
> explain more eloquently why it works.

FYI, the reason I shelved this one to reply to later was that you
didn't explain how it works.  It's much harder to review a patch for
correctness in that case.  I have to go figure it out myself
whether it's fixing the symptom or the cause.

When I went to do that I decided that the patch only fixed the symptom,
unfortunately.  Try repeating your test case, but instead of saying
"set edit off", say "-gdb-set edit off".  That won't switch
interpreters because it isn't running a CLI command.  Then try
-exec-next and it will fail.

The problem occurs because it's futzing with the CLI input handlers
when they are not installed, particularly input_handler.  I don't
really understand how all the event loop bits work.  The right solution
to this may be to not change anything if the current interpreter isn't
the CLI, but I don't know what should happen if TUI is enabled.  Or
maybe the right solution is to not change input_handler in
change_line_handler, just the other two.  The comment says to do
that in case "set edit off" is in .gdbinit, but I don't see why
that matters; do you?

The patch itself is probably good, by the way, just not for this
bug.  I would definitely like to merge the async changes in pieces,
so if you can explain why this patch is necessary and write a changelog
for it, I'll take a second look at it on its own merits.

> I don't know how to give attribution to these changes in the ChangeLog as I
> can't match them to entries in Apple's.  Perhaps Jim can provide the details.
> Otherwise can I just write?:
> 
> 2006-10-19  Apple Computer, Inc  <www.apple.com>
> 
> Or do I need a person/e-mail address?

I think it would be best to put your own name on the changelog, and
mention something like "From Apple Computer, Inc." at the top of the
entry.  I don't really know though so if anyone else has an opinion,
please share.

-- 
Daniel Jacobowitz
CodeSourcery


  reply	other threads:[~2006-11-17 21:35 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-10-19  8:59 Nick Roberts
2006-11-17 21:35 ` Daniel Jacobowitz [this message]
2006-11-20  2:37   ` Nick Roberts
2006-11-20  4:20     ` Daniel Jacobowitz
2006-11-20 22:25       ` Nick Roberts

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20061117213449.GC13961@nevyn.them.org \
    --to=drow@false.org \
    --cc=gdb-patches@sources.redhat.com \
    --cc=nickrob@snap.net.nz \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox