Re: PATCH: Enable backwards search in SQL Box

From: Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
To: Dave Page <dpage(at)pgadmin(dot)org>
Cc: "J(dot)F(dot) Oster" <jinfroster(at)mail(dot)ru>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: PATCH: Enable backwards search in SQL Box
Date: 2014-03-10 09:43:00
Message-ID: CANxoLDeoOor0mDd2aGjYdv3+m=VHiuA5=RBZhXYv2cD+q_6j5g@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

On Mon, Mar 10, 2014 at 3:10 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:

> On Sat, Mar 8, 2014 at 5:15 PM, J.F. Oster <jinfroster(at)mail(dot)ru> wrote:
> > Hello Dave,
> >
> > Wednesday, February 26, 2014, 12:56:51 AM, you wrote:
> >
> > DP> Shift-F3 should always do the opposite of what the radio
> > DP> buttons are set to. So, by default with the Forward radio button
> > DP> selected, Shift-F3 will search backwards, whilst if the Backwards
> > DP> radio button is selected, Shift-F3 should search forwards.
> >
> > You are right. For example, SciTE does the same.
> >
> > According to the above implemented it in a simplier way.
> > Please see the patch.
>
> Thanks. Akshay, can you take another look please?
>

Sure.

>
> > By the way, I was puzzled why search dialog's settings are saved
> > sometimes and sometimes are not. Depending on dialog's close method:
> > 1. Pressing ESC or "Close" form button calls OnCancel() which does not
> > save;
> > 2. Pressing title [x] button or Alt-F4 (or similar OS hotkey) calls
> > OnClose() which does save.
> >
> > Shouldn't the behaviour be the same regardless of dialog's close
> > method? Or at least the difference should be noted in documentation.
>
> Yes, it should. Patches welcome :-)
>
> > Wednesday, February 26, 2014, 12:56:51 AM, you wrote:
> >
> > DP> On Tue, Feb 25, 2014 at 9:38 AM, Akshay Joshi
> > DP> <akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
> > DP> Hi Dave═
> >
> >
> > DP> Ashesh asked me to review the patch. I have reviewed the
> > DP> patch and tested it. Found one behavior which gets change after
> > DP> applying the patch.
> >
> >
> > DP> Behavior Before:- When search direction is forward and user
> > DP> press F3, it searches the string in forward direction. Similarly
> > DP> if search direction is backward and user press F3, it searches the
> > DP> string in the backward direction. Nothing will be happen when user
> > DP> press═Shift-F3.═
> >
> >
> > DP> Behavior After :- ═When user press F3, it searches the string
> > DP> in forward direction while on pressing Shift-F3, it searches the
> > DP> string in backward direction. It is not dependent on the search
> > DP> direction selected from the dialog. Also the selection of radio
> > DP> button is dependent on the key press, if F3 is pressed then
> > DP> forward button is selected and if Shift-F3 is pressed then
> > DP> backward is selected.═
> >
> >
> > DP> Is this behavior acceptable?═
> >
> >
> >
> >
> >
> > DP> No, I don't think so (if I'm understanding correctly). I
> > DP> believe what should happen is that:
> >
> >
> > DP> - F3 should honour the search direction on the radio buttons,
> > DP> in all cases. If the Forward button is selected, F3 searches
> > DP> forward, and if the Backwards button is selected, F3 searches
> > DP> backwards.
> >
> >
> > DP> - Shift-F3 should always do the opposite of what the radio
> > DP> buttons are set to. So, by default with the Forward radio button
> > DP> selected, Shift-F3 will search backwards, whilst if the Backwards
> > DP> radio button is selected, Shift-F3 should search forwards.
> >
> >
> > DP> The reason for this is that the radio buttons give the most
> > DP> obvious visual cue, so F3 (Find Next) should follow what they are
> > DP> set to.
> > DP> ═
> >
> >
> > DP> On Thu, Feb 20, 2014 at 5:09 PM, Ashesh Vashi
> > DP> <ashesh(dot)vashi(at)enterprisedb(dot)com> wrote:
> > DP> Sure.
> > DP> I will do that.
> >
> >
> >
> > DP> On Thu, Feb 20, 2014 at 4:52 PM, Dave Page <dpage(at)pgadmin(dot)org>
> wrote:
> > DP> Ashesh, can you look at, and if appropriate, commit this please?
> >
> > DP> Thanks.
> >
> > DP> On Mon, Feb 17, 2014 at 5:46 PM, J.F. Oster <jinfroster(at)mail(dot)ru>
> wrote:
> >>> Hello!
> >>>
> >>> Pressing F3 in query editor makes the previously started search
> >>> continue forwards.
> >>> This patch adds expected behavior for Shift-F3.
> >>>
> >>> There is a minor inconvenience when just changing the search direction
> >>> - you have to press (Shift-)F3 *twice* for the (previous)next occurence
> >>> to get highlighted. That's because first time goes for cursor movement
> >>> to the corresponding boundary of current occurence. But that also
> >>> happens when search direction is changed with dialog radio-buttons, so
> >>> I think it's ok.
> >>>
> >
> >
> > --
> > Best regards,
> > J.F.
>
>
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

--
*Akshay Joshi*
*Principal Software Engineer *

*Phone: +91 20-3058-9517Mobile: +91 976-788-8246*

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Ashesh Vashi 2014-03-11 04:41:20 Re: debugger.c tries to set log_min_messages, why?
Previous Message Dave Page 2014-03-10 09:40:02 Re: PATCH: Enable backwards search in SQL Box