public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* Substitute '\' with '\\' in extended-prompt
@ 2014-10-17  4:13 Yao Qi
  2014-10-17  8:28 ` Phil Muldoon
  0 siblings, 1 reply; 9+ messages in thread
From: Yao Qi @ 2014-10-17  4:13 UTC (permalink / raw)
  To: Phil Muldoon; +Cc: gdb-patches


Hi Phil,
Why do we do the substitute in extended-prompt in your extended-prompt
patch <https://sourceware.org/ml/gdb-patches/2011-08/msg00236.html> as
below?

> +    def before_prompt_hook(self, current):
> +        if self.value is not '':
> +            newprompt = gdb.prompt.substitute_prompt(self.value)
> +            return newprompt.replace('\\', '\\\\')
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> +        else:
> +            return None

I don't see any explanations on this in comments or email.

Such substitute makes the output of "set extended-prompt \w" different
from the output of "pwd" nor "os.getcwdu()"

(gdb) python print os.getcwdu()^M
\\build2-lucid-cs\yqi\yqi\arm-none-eabi

(gdb) pwd^M
Working directory \\build2-lucid-cs\yqi\yqi\arm-none-eabi

(gdb) set extended-prompt \w
\\\\build2-lucid-cs\\yqi\\yqi\\arm-none-eabi

My patch <https://sourceware.org/ml/gdb-patches/2014-10/msg00423.html>
doesn't work due to this difference.  I'd like to know whether the
substitute is necessary or not.

-- 
Yao (齐尧)

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

* Re: Substitute '\' with '\\' in extended-prompt
  2014-10-17  4:13 Substitute '\' with '\\' in extended-prompt Yao Qi
@ 2014-10-17  8:28 ` Phil Muldoon
  2014-10-17 11:23   ` Yao Qi
  0 siblings, 1 reply; 9+ messages in thread
From: Phil Muldoon @ 2014-10-17  8:28 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

On 17/10/14 05:09, Yao Qi wrote:
>
> Hi Phil,
> Why do we do the substitute in extended-prompt in your extended-prompt
> patch <https://sourceware.org/ml/gdb-patches/2011-08/msg00236.html> as
> below?
>
>> +    def before_prompt_hook(self, current):
>> +        if self.value is not '':
>> +            newprompt = gdb.prompt.substitute_prompt(self.value)
>> +            return newprompt.replace('\\', '\\\\')
>                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>> +        else:
>> +            return None
>
> I don't see any explanations on this in comments or email.

The lacks of a comment really bothers me!  The short answer is I can't
remember why this is needed.  I think it likely that if "\\" is passed
to readline it is parsed out.

See from the documentation:

+@item @var{\\}
+Substitute a backslash.

So the additional escape sequences are needed (for readline) after the
prompt command substitution.  I think if memory serves, to escape an
escape character, you need the four "\"'s but in the prompt command
language we only specify two.

However I have not looked at it in detail yet, but that is my first
guess. If you remove it, do the Python prompt tests fail?  I'll do
some archaeology and try to find out why today.  However, if it is
blocking your work, and without the substitution all the tests still
pass, then I would suggest we remove it for now.  We can always add it
back, and the next release of GDB is not terribly soon.

Cheers

Phil




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

* Re: Substitute '\' with '\\' in extended-prompt
  2014-10-17  8:28 ` Phil Muldoon
@ 2014-10-17 11:23   ` Yao Qi
  2014-10-22 13:01     ` [PATCH] Don't replace '\' with '\\' in before_prompt_hook Yao Qi
  0 siblings, 1 reply; 9+ messages in thread
From: Yao Qi @ 2014-10-17 11:23 UTC (permalink / raw)
  To: Phil Muldoon; +Cc: gdb-patches

Phil Muldoon <pmuldoon@redhat.com> writes:

> However I have not looked at it in detail yet, but that is my first
> guess. If you remove it, do the Python prompt tests fail?  I'll do

I remove it, and no python tests fail.  The output of
"set extended-prompt \w" changed from
"\\\\build2-lucid-cs\\yqi\\yqi\\arm-none-eabi" to
"\\build2-lucid-cs\yqi\yqi\arm-none-eabi".  I did the test on mingw32
host.

> some archaeology and try to find out why today.  However, if it is
> blocking your work, and without the substitution all the tests still
> pass, then I would suggest we remove it for now.  We can always add it
> back, and the next release of GDB is not terribly soon.

I am in no hurry.  I can post a patch to remove it if we don't have any
clues in three days.

-- 
Yao (齐尧)

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

* [PATCH] Don't replace '\' with '\\' in before_prompt_hook
  2014-10-17 11:23   ` Yao Qi
@ 2014-10-22 13:01     ` Yao Qi
  2014-10-29 14:23       ` Yao Qi
  0 siblings, 1 reply; 9+ messages in thread
From: Yao Qi @ 2014-10-22 13:01 UTC (permalink / raw)
  To: gdb-patches

In gdb/command/prompt.py:before_prompt_hook, the '\' in the new prompt
is replaced with '\\', shown as below,

>     def before_prompt_hook(self, current):
>         if self.value is not '':
>             newprompt = gdb.prompt.substitute_prompt(self.value)
>             return newprompt.replace('\\', '\\\\')
>         else:
>             return None

I don't see any explanations on this in comments nor email.  As doc
said, "set extended-prompt \w" substitute the current working
directory, but it prints something different from what pwd or
os.getcwdu() prints on mingw32 host.

(gdb) python print os.getcwdu()^M
\\build2-lucid-cs\yqi\yqi\arm-none-eabi

(gdb) pwd^M
Working directory \\build2-lucid-cs\yqi\yqi\arm-none-eabi

(gdb) set extended-prompt \w
\\\\build2-lucid-cs\\yqi\\yqi\\arm-none-eabi

This makes me think whether the substitution in before_prompt_hook is
necessary or not.  This patch is to remove this substitution.

Run gdb.python on x86_64-linux and arm-none-eabi on mingw32 host.  No
regressions.  Is it OK?

gdb:

2014-10-22  Yao Qi  <yao@codesourcery.com>

	* python/lib/gdb/command/prompt.py (before_prompt_hook): Don't
	replace '\\' with '\\\\'.:
---
 gdb/python/lib/gdb/command/prompt.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/gdb/python/lib/gdb/command/prompt.py b/gdb/python/lib/gdb/command/prompt.py
index e7dc3da..5e973e4 100644
--- a/gdb/python/lib/gdb/command/prompt.py
+++ b/gdb/python/lib/gdb/command/prompt.py
@@ -58,8 +58,7 @@ The currently defined substitutions are:
 
     def before_prompt_hook(self, current):
         if self.value is not '':
-            newprompt = gdb.prompt.substitute_prompt(self.value)
-            return newprompt.replace('\\', '\\\\')
+            return gdb.prompt.substitute_prompt(self.value)
         else:
             return None
 
-- 
1.9.3

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

* Re: [PATCH] Don't replace '\' with '\\' in before_prompt_hook
  2014-10-22 13:01     ` [PATCH] Don't replace '\' with '\\' in before_prompt_hook Yao Qi
@ 2014-10-29 14:23       ` Yao Qi
  2014-10-29 14:34         ` Phil Muldoon
  0 siblings, 1 reply; 9+ messages in thread
From: Yao Qi @ 2014-10-29 14:23 UTC (permalink / raw)
  To: gdb-patches

Yao Qi <yao@codesourcery.com> writes:

> In gdb/command/prompt.py:before_prompt_hook, the '\' in the new prompt
> is replaced with '\\', shown as below,
>
>>     def before_prompt_hook(self, current):
>>         if self.value is not '':
>>             newprompt = gdb.prompt.substitute_prompt(self.value)
>>             return newprompt.replace('\\', '\\\\')
>>         else:
>>             return None
>
> I don't see any explanations on this in comments nor email.  As doc
> said, "set extended-prompt \w" substitute the current working
> directory, but it prints something different from what pwd or
> os.getcwdu() prints on mingw32 host.
>
> (gdb) python print os.getcwdu()^M
> \\build2-lucid-cs\yqi\yqi\arm-none-eabi
>
> (gdb) pwd^M
> Working directory \\build2-lucid-cs\yqi\yqi\arm-none-eabi
>
> (gdb) set extended-prompt \w
> \\\\build2-lucid-cs\\yqi\\yqi\\arm-none-eabi
>
> This makes me think whether the substitution in before_prompt_hook is
> necessary or not.  This patch is to remove this substitution.
>
> Run gdb.python on x86_64-linux and arm-none-eabi on mingw32 host.  No
> regressions.  Is it OK?
>
> gdb:
>
> 2014-10-22  Yao Qi  <yao@codesourcery.com>
>
> 	* python/lib/gdb/command/prompt.py (before_prompt_hook): Don't
> 	replace '\\' with '\\\\'.:

The trailing : can be removed.  I appreciate people familiar with gdb
python can review this patch, so, here is a ping.

-- 
Yao (齐尧)

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

* Re: [PATCH] Don't replace '\' with '\\' in before_prompt_hook
  2014-10-29 14:23       ` Yao Qi
@ 2014-10-29 14:34         ` Phil Muldoon
  2014-10-29 14:39           ` Phil Muldoon
  0 siblings, 1 reply; 9+ messages in thread
From: Phil Muldoon @ 2014-10-29 14:34 UTC (permalink / raw)
  To: Yao Qi, gdb-patches

On 29/10/14 14:19, Yao Qi wrote:
> Yao Qi <yao@codesourcery.com> writes:
>
>> In gdb/command/prompt.py:before_prompt_hook, the '\' in the new prompt
>> is replaced with '\\', shown as below,
>>
>>>     def before_prompt_hook(self, current):
>>>         if self.value is not '':
>>>             newprompt = gdb.prompt.substitute_prompt(self.value)
>>>             return newprompt.replace('\\', '\\\\')
>>>         else:
>>>             return None
>>
>> I don't see any explanations on this in comments nor email.  As doc
>> said, "set extended-prompt \w" substitute the current working
>> directory, but it prints something different from what pwd or
>> os.getcwdu() prints on mingw32 host.
>>
>> (gdb) python print os.getcwdu()^M
>> \\build2-lucid-cs\yqi\yqi\arm-none-eabi
>>
>> (gdb) pwd^M
>> Working directory \\build2-lucid-cs\yqi\yqi\arm-none-eabi
>>
>> (gdb) set extended-prompt \w
>> \\\\build2-lucid-cs\\yqi\\yqi\\arm-none-eabi
>>
>> This makes me think whether the substitution in before_prompt_hook is
>> necessary or not.  This patch is to remove this substitution.
>>
>> Run gdb.python on x86_64-linux and arm-none-eabi on mingw32 host.  No
>> regressions.  Is it OK?
>>
>> gdb:
>>
>> 2014-10-22  Yao Qi  <yao@codesourcery.com>
>>
>>     * python/lib/gdb/command/prompt.py (before_prompt_hook): Don't
>>     replace '\\' with '\\\\'.:
>
> The trailing : can be removed.  I appreciate people familiar with gdb
> python can review this patch, so, here is a ping.
>

If the tests pass, I think it is fine.  We are not close to a release,
so we can put it back if it causes issues (and fix up some new tests
to catch the regression.)

Cheers

Phil

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

* Re: [PATCH] Don't replace '\' with '\\' in before_prompt_hook
  2014-10-29 14:34         ` Phil Muldoon
@ 2014-10-29 14:39           ` Phil Muldoon
  2014-10-29 23:37             ` Yao Qi
  0 siblings, 1 reply; 9+ messages in thread
From: Phil Muldoon @ 2014-10-29 14:39 UTC (permalink / raw)
  To: Yao Qi, gdb-patches

On 29/10/14 14:34, Phil Muldoon wrote:
> On 29/10/14 14:19, Yao Qi wrote:
>> Yao Qi <yao@codesourcery.com> writes:
>>
>>> In gdb/command/prompt.py:before_prompt_hook, the '\' in the new prompt
>>> is replaced with '\\', shown as below,
>>>
>>>>     def before_prompt_hook(self, current):
>>>>         if self.value is not '':
>>>>             newprompt = gdb.prompt.substitute_prompt(self.value)
>>>>             return newprompt.replace('\\', '\\\\')
>>>>         else:
>>>>             return None
>>> I don't see any explanations on this in comments nor email.  As doc
>>> said, "set extended-prompt \w" substitute the current working
>>> directory, but it prints something different from what pwd or
>>> os.getcwdu() prints on mingw32 host.
>>>
>>> (gdb) python print os.getcwdu()^M
>>> \\build2-lucid-cs\yqi\yqi\arm-none-eabi
>>>
>>> (gdb) pwd^M
>>> Working directory \\build2-lucid-cs\yqi\yqi\arm-none-eabi
>>>
>>> (gdb) set extended-prompt \w
>>> \\\\build2-lucid-cs\\yqi\\yqi\\arm-none-eabi
>>>
>>> This makes me think whether the substitution in before_prompt_hook is
>>> necessary or not.  This patch is to remove this substitution.
>>>
>>> Run gdb.python on x86_64-linux and arm-none-eabi on mingw32 host.  No
>>> regressions.  Is it OK?
>>>
>>> gdb:
>>>
>>> 2014-10-22  Yao Qi  <yao@codesourcery.com>
>>>
>>>     * python/lib/gdb/command/prompt.py (before_prompt_hook): Don't
>>>     replace '\\' with '\\\\'.:
>> The trailing : can be removed.  I appreciate people familiar with gdb
>> python can review this patch, so, here is a ping.
>>
> If the tests pass, I think it is fine.  We are not close to a release,
> so we can put it back if it causes issues (and fix up some new tests
> to catch the regression.)
>
Oops I better clarify, this is for mainline, not the impending 7.8.1 release?

Cheers

Phil

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

* Re: [PATCH] Don't replace '\' with '\\' in before_prompt_hook
  2014-10-29 14:39           ` Phil Muldoon
@ 2014-10-29 23:37             ` Yao Qi
  2014-10-30  1:48               ` Yao Qi
  0 siblings, 1 reply; 9+ messages in thread
From: Yao Qi @ 2014-10-29 23:37 UTC (permalink / raw)
  To: Phil Muldoon; +Cc: gdb-patches

Phil Muldoon <pmuldoon@redhat.com> writes:

>> If the tests pass, I think it is fine.  We are not close to a release,
>> so we can put it back if it causes issues (and fix up some new tests
>> to catch the regression.)
>>
> Oops I better clarify, this is for mainline, not the impending 7.8.1 release?

Phil,
this patch is for mainline.

-- 
Yao (齐尧)

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

* Re: [PATCH] Don't replace '\' with '\\' in before_prompt_hook
  2014-10-29 23:37             ` Yao Qi
@ 2014-10-30  1:48               ` Yao Qi
  0 siblings, 0 replies; 9+ messages in thread
From: Yao Qi @ 2014-10-30  1:48 UTC (permalink / raw)
  To: Phil Muldoon; +Cc: gdb-patches

Yao Qi <yao@codesourcery.com> writes:

>>> If the tests pass, I think it is fine.  We are not close to a release,
>>> so we can put it back if it causes issues (and fix up some new tests
>>> to catch the regression.)
>>>
>> Oops I better clarify, this is for mainline, not the impending 7.8.1 release?
>
> Phil,
> this patch is for mainline.

I've pushed it in to mainline.

-- 
Yao (齐尧)

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

end of thread, other threads:[~2014-10-30  1:48 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-17  4:13 Substitute '\' with '\\' in extended-prompt Yao Qi
2014-10-17  8:28 ` Phil Muldoon
2014-10-17 11:23   ` Yao Qi
2014-10-22 13:01     ` [PATCH] Don't replace '\' with '\\' in before_prompt_hook Yao Qi
2014-10-29 14:23       ` Yao Qi
2014-10-29 14:34         ` Phil Muldoon
2014-10-29 14:39           ` Phil Muldoon
2014-10-29 23:37             ` Yao Qi
2014-10-30  1:48               ` Yao Qi

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