Обсуждение: Fwd: Pgadmin 1.12 rc1 editing bug

 Hi,

Under WinXp Sp3 try the following:
- open a table data in a grid
- open a cell for editing
- select some text
- press the 'del' key.
- the selected text doesn't deleted

Regards,   Ferenc

-- 
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers

  Hi Ferenc,

'Del' is disabled because of it's reserved for 'drop\del' rows in this
situation.
I don't know if it makes sense to change this behaviour.
May be Dave or Guillaume have another opinion of that.

Regards Steffen

On 03.09.2010 16:10, Steffen Kuhn wrote:
> Hi Ferenc,
>
> the same one Ubuntu.
> 'Del' is disabled for some reasons in the edit menu.
> I'll try to track this down.
>
> Regards Steffen
>
> -------- Original-Nachricht --------
> Betreff:     [pgadmin-hackers] Pgadmin 1.12 rc1 editing bug
> Datum:     Fri, 03 Sep 2010 13:38:09 +0200
> Von:     Lutischán Ferenc <lutischanf@gmail.com>
> An:     pgadmin-hackers@postgresql.org
>
>
>
>   Hi,
>
> Under WinXp Sp3 try the following:
> - open a table data in a grid
> - open a cell for editing
> - select some text
> - press the 'del' key.
> - the selected text doesn't deleted
>
> Regards,
>      Ferenc
>
> --
> Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgadmin-hackers
>

Hi Ferenc,

the same one Ubuntu.
'Del' is disabled for some reasons in the edit menu.
I'll try to track this down.

Regards Steffen

-------- Original-Nachricht --------

Betreff:	[pgadmin-hackers] Pgadmin 1.12 rc1 editing bug
Datum:	Fri, 03 Sep 2010 13:38:09 +0200
Von:	Lutischán Ferenc <lutischanf@gmail.com>
An:	pgadmin-hackers@postgresql.org

 Hi,

Under WinXp Sp3 try the following:
- open a table data in a grid
- open a cell for editing
- select some text
- press the 'del' key.
- the selected text doesn't deleted

Regards,   Ferenc

-- 
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers

Le 03/09/2010 16:37, Steffen Kuhn a écrit :
>  Hi Ferenc,
>
> 'Del' is disabled because of it's reserved for 'drop\del' rows in this
> situation.
> I don't know if it makes sense to change this behaviour.
> May be Dave or Guillaume have another opinion of that.
>

My opinion is that the DEL key should work :)

It should have the usual behaviour when an editor is open, it should
delete the row otherwise.

Tell me if you want to work on it, I have enough work to do on the GSoC.
Otherwise, I'll take care of it.


--
Guillaume
 http://www.postgresql.fr
 http://dalibo.com

  Okay I'll do my best. ;-)

Did I get you right to just perform a delete for to current text
selection or the letter after the cursor
and not enabling menu and toolbar (commented with 'Delete selected rows.') ?

Regards Steffen

On 03.09.2010 16:52, Guillaume Lelarge wrote:
> Le 03/09/2010 16:37, Steffen Kuhn a écrit :
>>   Hi Ferenc,
>>
>> 'Del' is disabled because of it's reserved for 'drop\del' rows in this
>> situation.
>> I don't know if it makes sense to change this behaviour.
>> May be Dave or Guillaume have another opinion of that.
>>
> My opinion is that the DEL key should work :)
>
> It should have the usual behaviour when an editor is open, it should
> delete the row otherwise.
>
> Tell me if you want to work on it, I have enough work to do on the GSoC.
> Otherwise, I'll take care of it.
>
>

Le 03/09/2010 17:08, Steffen Kuhn a écrit :
>  Okay I'll do my best. ;-)
>

Thanks. That gives me more time for the GSoC. I really appreciate that.

> Did I get you right to just perform a delete for to current text
> selection or the letter after the cursor

Yes.

> and not enabling menu and toolbar (commented with 'Delete selected
> rows.') ?
>

The icon on the toolbar is not at fault here. It's a delete row button,
and it should stay that way.

The Delete item in the Edit menu should be enabled when an editor is
open. And disabled otherwise.


--
Guillaume
 http://www.postgresql.fr
 http://dalibo.com

  Hi Guillaume,

the delete problem is following:
deleting rows has got the shortcut 'del'. Disabeling row deletion
disables 'del'.
Even the key event WXK_DELETE is not fired.
So the solution of this problem which should been there since 'del' is
the shortcut for row delete
is to change the shortcut to eg. 'ctrl-del', to get the default behavior
of the del key.
Could you please comment this.

Working on this I located other issues in the edit context:
_Select some text use context menu an try to perform an action.
Nothing happens, because as I assume selection and focus is lost while
opening the conext menu.
_Select a numeric value and use context menu. A totally different
context menu occurs (system context menu includeing unicode input
features, icons etc.),
which I know from situations in which no menu was defined.

For the shortcut issue I attach a patch.
The new rest will take some time, because for newbie eyes this looks not
trivial.

Regards Steffen

On 03.09.2010 17:12, Guillaume Lelarge wrote:
> Le 03/09/2010 17:08, Steffen Kuhn a écrit :
>>   Okay I'll do my best. ;-)
>>
> Thanks. That gives me more time for the GSoC. I really appreciate that.
>
>> Did I get you right to just perform a delete for to current text
>> selection or the letter after the cursor
> Yes.
>
>> and not enabling menu and toolbar (commented with 'Delete selected
>> rows.') ?
>>
> The icon on the toolbar is not at fault here. It's a delete row button,
> and it should stay that way.
>
> The Delete item in the Edit menu should be enabled when an editor is
> open. And disabled otherwise.
>
>

On Fri, Sep 3, 2010 at 9:24 PM, Steffen Kuhn <pg@kuhnsteffen.de> wrote:
>  Hi Guillaume,
>
> the delete problem is following:
> deleting rows has got the shortcut 'del'. Disabeling row deletion disables
> 'del'.
> Even the key event WXK_DELETE is not fired.
> So the solution of this problem which should been there since 'del' is the
> shortcut for row delete
> is to change the shortcut to eg. 'ctrl-del', to get the default behavior of
> the del key.
> Could you please comment this.

Del on its own is the correct key. It should be able to do the "right
thing" based on whether or not a cell is in edit mode.



--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise Postgres Company

Le 03/09/2010 23:07, Dave Page a écrit :
> On Fri, Sep 3, 2010 at 9:24 PM, Steffen Kuhn <pg@kuhnsteffen.de> wrote:
>>  Hi Guillaume,
>>
>> the delete problem is following:
>> deleting rows has got the shortcut 'del'. Disabeling row deletion disables
>> 'del'.
>> Even the key event WXK_DELETE is not fired.
>> So the solution of this problem which should been there since 'del' is the
>> shortcut for row delete
>> is to change the shortcut to eg. 'ctrl-del', to get the default behavior of
>> the del key.
>> Could you please comment this.
>
> Del on its own is the correct key. It should be able to do the "right
> thing" based on whether or not a cell is in edit mode.
>

Yeah, I agree. See patch attached (not yet commited).


--
Guillaume
 http://www.postgresql.fr
 http://dalibo.com

On Fri, Sep 3, 2010 at 11:39 PM, Guillaume Lelarge
<guillaume@lelarge.info> wrote:
> Le 03/09/2010 23:07, Dave Page a écrit :
>> On Fri, Sep 3, 2010 at 9:24 PM, Steffen Kuhn <pg@kuhnsteffen.de> wrote:
>>>  Hi Guillaume,
>>>
>>> the delete problem is following:
>>> deleting rows has got the shortcut 'del'. Disabeling row deletion disables
>>> 'del'.
>>> Even the key event WXK_DELETE is not fired.
>>> So the solution of this problem which should been there since 'del' is the
>>> shortcut for row delete
>>> is to change the shortcut to eg. 'ctrl-del', to get the default behavior of
>>> the del key.
>>> Could you please comment this.
>>
>> Del on its own is the correct key. It should be able to do the "right
>> thing" based on whether or not a cell is in edit mode.
>>
>
> Yeah, I agree. See patch attached (not yet commited).

Hmm, that seems to be setting up the button to handle deletion of
text, which is not really consistent with anything else. Can we leave
the button to be enabled/disabled as it currently is, and ensure the
right thing is done in edit more? That would also avoid breaking a
translation...

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise Postgres Company

Hi Dave,

this was the way I tried to solve the issue.
Do not change the enabled/disabled state of the button but getting the back 'del' function.
As I see the only and minor invasive way is to change the current delete row shortcut to 'ctrl-del'.
BTW this did not break the german translation.

Regards Steffen

On 06.09.2010 09:39 "Dave Page" <dpage@pgadmin.org> wrote:
> On Fri, Sep 3, 2010 at 11:39 PM, Guillaume Lelarge
> <guillaume@lelarge.info> wrote:
> > Le 03/09/2010 23:07, Dave Page a écrit :
> >> On Fri, Sep 3, 2010 at 9:24 PM, Steffen Kuhn <pg@kuhnsteffen.de>
> >> wrote:
> >>>  Hi Guillaume,
> >>>
> >>> the delete problem is following:
> >>> deleting rows has got the shortcut 'del'. Disabeling row deletion
> >>> disables
> >>> 'del'.
> >>> Even the key event WXK_DELETE is not fired.
> >>> So the solution of this problem which should been there since
> >>> 'del' is the
> >>> shortcut for row delete
> >>> is to change the shortcut to eg. 'ctrl-del', to get the default
> >>> behavior of
> >>> the del key.
> >>> Could you please comment this.
> >>
> >> Del on its own is the correct key. It should be able to do the
> >> "right
> >> thing" based on whether or not a cell is in edit mode.
> >>
> >
> > Yeah, I agree. See patch attached (not yet commited).
>
> Hmm, that seems to be setting up the button to handle deletion of
> text, which is not really consistent with anything else. Can we leave
> the button to be enabled/disabled as it currently is, and ensure the
> right thing is done in edit more? That would also avoid breaking a
> translation...
>

Le 06/09/2010 09:39, Dave Page a écrit :
> On Fri, Sep 3, 2010 at 11:39 PM, Guillaume Lelarge
> <guillaume@lelarge.info> wrote:
>> Le 03/09/2010 23:07, Dave Page a écrit :
>>> On Fri, Sep 3, 2010 at 9:24 PM, Steffen Kuhn <pg@kuhnsteffen.de> wrote:
>>>>  Hi Guillaume,
>>>>
>>>> the delete problem is following:
>>>> deleting rows has got the shortcut 'del'. Disabeling row deletion disables
>>>> 'del'.
>>>> Even the key event WXK_DELETE is not fired.
>>>> So the solution of this problem which should been there since 'del' is the
>>>> shortcut for row delete
>>>> is to change the shortcut to eg. 'ctrl-del', to get the default behavior of
>>>> the del key.
>>>> Could you please comment this.
>>>
>>> Del on its own is the correct key. It should be able to do the "right
>>> thing" based on whether or not a cell is in edit mode.
>>>
>>
>> Yeah, I agree. See patch attached (not yet commited).
>
> Hmm, that seems to be setting up the button to handle deletion of
> text, which is not really consistent with anything else. Can we leave
> the button to be enabled/disabled as it currently is, and ensure the
> right thing is done in edit more? That would also avoid breaking a
> translation...
>

It's possible to disable the toolbar button, but we'll need to keep the
menu item enabled so that the DEL key still triggers the OnDelete()
method. Is this what you want?


--
Guillaume
 http://www.postgresql.fr
 http://dalibo.com

Le 06/09/2010 12:09, Steffen Kuhn a écrit :
> Hi Dave,
>
> this was the way I tried to solve the issue.

Nope; You changed the shortcut, so that the DEL key could be used with
the textboxes.

> Do not change the enabled/disabled state of the button but getting the back 'del' function.
> As I see the only and minor invasive way is to change the current delete row shortcut to 'ctrl-del'.
> BTW this did not break the german translation.
>

It should have break the german translation as you modified a
translatable string.


--
Guillaume
 http://www.postgresql.fr
 http://dalibo.com

On Mon, Sep 6, 2010 at 5:03 PM, Guillaume Lelarge
<guillaume@lelarge.info> wrote:
> It's possible to disable the toolbar button, but we'll need to keep the
> menu item enabled so that the DEL key still triggers the OnDelete()
> method. Is this what you want?

What about adding an accelerator for the DEL key binding, so it
doesn't matter if the menu is disabled or not?


--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise Postgres Company

Le 06/09/2010 18:13, Dave Page a écrit :
> On Mon, Sep 6, 2010 at 5:03 PM, Guillaume Lelarge
> <guillaume@lelarge.info> wrote:
>> It's possible to disable the toolbar button, but we'll need to keep the
>> menu item enabled so that the DEL key still triggers the OnDelete()
>> method. Is this what you want?
>
> What about adding an accelerator for the DEL key binding, so it
> doesn't matter if the menu is disabled or not?
>

There's already an accelerator entry for the DEL key. If the menu is
disabled, the method is not fired. So, I only know two solutions: don't
disable the menu item, don't show the Del keybinding in the menu item.


--
Guillaume
 http://www.postgresql.fr
 http://dalibo.com

On Mon, Sep 6, 2010 at 5:34 PM, Guillaume Lelarge
<guillaume@lelarge.info> wrote:
> Le 06/09/2010 18:13, Dave Page a écrit :
>> On Mon, Sep 6, 2010 at 5:03 PM, Guillaume Lelarge
>> <guillaume@lelarge.info> wrote:
>>> It's possible to disable the toolbar button, but we'll need to keep the
>>> menu item enabled so that the DEL key still triggers the OnDelete()
>>> method. Is this what you want?
>>
>> What about adding an accelerator for the DEL key binding, so it
>> doesn't matter if the menu is disabled or not?
>>
>
> There's already an accelerator entry for the DEL key. If the menu is
> disabled, the method is not fired. So, I only know two solutions: don't
> disable the menu item, don't show the Del keybinding in the menu item.

Does creating the accelerator table before the menu make any difference?



--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise Postgres Company

Le 06/09/2010 18:37, Dave Page a écrit :
> On Mon, Sep 6, 2010 at 5:34 PM, Guillaume Lelarge
> <guillaume@lelarge.info> wrote:
>> Le 06/09/2010 18:13, Dave Page a écrit :
>>> On Mon, Sep 6, 2010 at 5:03 PM, Guillaume Lelarge
>>> <guillaume@lelarge.info> wrote:
>>>> It's possible to disable the toolbar button, but we'll need to keep the
>>>> menu item enabled so that the DEL key still triggers the OnDelete()
>>>> method. Is this what you want?
>>>
>>> What about adding an accelerator for the DEL key binding, so it
>>> doesn't matter if the menu is disabled or not?
>>>
>>
>> There's already an accelerator entry for the DEL key. If the menu is
>> disabled, the method is not fired. So, I only know two solutions: don't
>> disable the menu item, don't show the Del keybinding in the menu item.
>
> Does creating the accelerator table before the menu make any difference?
>

Nope, doesn't work.


--
Guillaume
 http://www.postgresql.fr
 http://dalibo.com

On Mon, Sep 6, 2010 at 5:44 PM, Guillaume Lelarge
<guillaume@lelarge.info> wrote:
> Le 06/09/2010 18:37, Dave Page a écrit :
>> On Mon, Sep 6, 2010 at 5:34 PM, Guillaume Lelarge
>> <guillaume@lelarge.info> wrote:
>>> Le 06/09/2010 18:13, Dave Page a écrit :
>>>> On Mon, Sep 6, 2010 at 5:03 PM, Guillaume Lelarge
>>>> <guillaume@lelarge.info> wrote:
>>>>> It's possible to disable the toolbar button, but we'll need to keep the
>>>>> menu item enabled so that the DEL key still triggers the OnDelete()
>>>>> method. Is this what you want?
>>>>
>>>> What about adding an accelerator for the DEL key binding, so it
>>>> doesn't matter if the menu is disabled or not?
>>>>
>>>
>>> There's already an accelerator entry for the DEL key. If the menu is
>>> disabled, the method is not fired. So, I only know two solutions: don't
>>> disable the menu item, don't show the Del keybinding in the menu item.
>>
>> Does creating the accelerator table before the menu make any difference?
>>
>
> Nope, doesn't work.

I say we remove the shortcut from the menu option then.


--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise Postgres Company

Le 06/09/2010 21:11, Dave Page a écrit :
> On Mon, Sep 6, 2010 at 5:44 PM, Guillaume Lelarge
> <guillaume@lelarge.info> wrote:
>> Le 06/09/2010 18:37, Dave Page a écrit :
>>> On Mon, Sep 6, 2010 at 5:34 PM, Guillaume Lelarge
>>> <guillaume@lelarge.info> wrote:
>>>> Le 06/09/2010 18:13, Dave Page a écrit :
>>>>> On Mon, Sep 6, 2010 at 5:03 PM, Guillaume Lelarge
>>>>> <guillaume@lelarge.info> wrote:
>>>>>> It's possible to disable the toolbar button, but we'll need to keep the
>>>>>> menu item enabled so that the DEL key still triggers the OnDelete()
>>>>>> method. Is this what you want?
>>>>>
>>>>> What about adding an accelerator for the DEL key binding, so it
>>>>> doesn't matter if the menu is disabled or not?
>>>>>
>>>>
>>>> There's already an accelerator entry for the DEL key. If the menu is
>>>> disabled, the method is not fired. So, I only know two solutions: don't
>>>> disable the menu item, don't show the Del keybinding in the menu item.
>>>
>>> Does creating the accelerator table before the menu make any difference?
>>>
>>
>> Nope, doesn't work.
>
> I say we remove the shortcut from the menu option then.
>
>

OK, then. Commited on 1.12 and master. Thanks.


--
Guillaume
 http://www.postgresql.fr
 http://dalibo.com