Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Jan Kratochvil <jan.kratochvil@redhat.com>
To: mbilal <mbilal@codesourcery.com>
Cc: gdb-patches@sourceware.org, tromey@redhat.com,
	       Pedro Alves <palves@redhat.com>
Subject: Re: Fwd: [PATCH] Fix for PR gdb/15224  should "set history save on" by default
Date: Wed, 03 Apr 2013 17:50:00 -0000	[thread overview]
Message-ID: <20130403143007.GA20343@host2.jankratochvil.net> (raw)
In-Reply-To: <515C3247.3050408@redhat.com>

Hello Muhammad,

as you asked on IRC for a summary of tasks to do.

These tasks should be posted as separate (independent if possible) patches.


(1) Fix relative "set history filename" to by immediately converted to absolute
    form:

On Wed, 03 Apr 2013 15:44:39 +0200, Pedro Alves wrote:
>  $ gdb
>  (gdb) show history filename
>  The filename in which to record the command history is "/tmp/.gdb_history".
>  (gdb) q
> 
> Notice, absolute path above.
> 
>  $ cd /tmp
>  $ mkdir a
>  $ gdb -ex "set history filename .gdbhist"
>  (gdb) show history filename
>  The filename in which to record the command history is ".gdbhist".
>  (gdb) cd a
>  Working directory /tmp/a.
>  (gdb) q
>  $ cat a/.gdbhist
>  show history filename
>  cd a
>  q
>  $ ls .gdbhist a/.gdbhist
>  ls: cannot access .gdbhist: No such file or directory
>  a/.gdbhist
>  $
> 
> So, the current default resolves the relative .gdb_history path
> early on, meaning, the history file written is the same that was
> read, while "set history filename .gdb_history" resolves to
> different files at read and at write times, as seen in the example
> above.  I'd call that a bug too.
+
> Not necessary it seems then.  The above issue should be addressed though.
> Probably by making "set history filename"'s set hook resolve the path.
> Whether to make "show history filename" show just the resolved path, or
> both, I'm undecided.


(2) Unify interactivity tests to use input_from_terminal_p:

On Mon, 01 Apr 2013 16:15:11 +0200, Pedro Alves wrote:
> We don't add commands to history unless we're interactive debugging:
> 
>   /* Add line to history if appropriate.  */
>   if (instream == stdin
>       && ISATTY (stdin) && *linebuffer)
>     add_history (linebuffer);
+
> Probably, it'd be best to use input_from_terminal_p, which also
> checks for --batch mode, among other things.


(3) Fix currently incorrectly touched history file even if no commands get
    added to it:

On Mon, 01 Apr 2013 16:15:11 +0200, Pedro Alves wrote:
> I think however, that whether to write history to file should also
> check whether we're in interactive debugging mode:
> 
>   /* Save the history information if it is appropriate to do so.  */
>   if (write_history_p && history_filename)
>     write_history (history_filename);
> 
> That is, e.g., this touches history:
> 
>  $ ls -als ~/.gdbhist
>  4 -rw-------. 1 pedro pedro 27 Apr  1 14:10 /home/pedro/.gdbhist
>  $ date
>  Mon Apr  1 14:37:01 WEST 2013
>  $ gdb -ex "set history filename ~/.gdbhist" < /dev/null
>  (gdb) quit
>  $ ls -als ~/.gdbhist
>  4 -rw-------. 1 pedro pedro 27 Apr  1 14:37 /home/pedro/.gdbhist
> 
> That, I'd call a bug.
> 
> Probably, it'd be best to use input_from_terminal_p, which also
> checks for --batch mode, among other things.


(4) Modify GDB testsuite to always disable history saving.

> As for testsuites and similar setups, I'm thinking the cases that could
> be affected would be restricted to setups like ours and gcc's, that drive
> gdb with expect, which is precisely designed to launch programs
> interactively, as if a user was running them, so it'd make a lot of sense
> to me to not treat them specially (having them adjust instead).


(5) No action required - MI command "-interpreter-exec console" does not save
    it to the history, it is questionable whether it should or not, it could
    be changed in the future.


(6) Change the default set history filename to ~/.gdb_history.

On Mon, 01 Apr 2013 14:29:30 +0200, Jan Kratochvil wrote:
> I see there should be also changed the default to:
>       set history filename ~/.gdb_history


(7) Enable "set history save on" by default as you already did in your patch.


Thanks,
Jan


      reply	other threads:[~2013-04-03 14:30 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <51558CB4.8010003@codesourcery.com>
2013-03-29 16:18 ` mbilal
2013-03-29 16:28   ` Jan Kratochvil
2013-04-01 11:34     ` Pedro Alves
2013-04-01 12:41       ` Jan Kratochvil
2013-04-01 19:44         ` Pedro Alves
2013-04-02 17:10           ` Jan Kratochvil
2013-04-03 17:14             ` Pedro Alves
2013-04-03 17:50               ` Jan Kratochvil [this message]

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=20130403143007.GA20343@host2.jankratochvil.net \
    --to=jan.kratochvil@redhat.com \
    --cc=gdb-patches@sourceware.org \
    --cc=mbilal@codesourcery.com \
    --cc=palves@redhat.com \
    --cc=tromey@redhat.com \
    /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