Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtp (Exim 4.84_2) (envelope-from ) id 1cQAEg-00066w-GP for pgadmin-hackers@arkaria.postgresql.org; Sun, 08 Jan 2017 09:57:58 +0000 Received: from localhost ([127.0.0.1] helo=postgresql.org) by malur.postgresql.org with smtp (Exim 4.84_2) (envelope-from ) id 1cQAEg-0002qp-3Q for pgadmin-hackers@arkaria.postgresql.org; Sun, 08 Jan 2017 09:57:58 +0000 Received: from makus.postgresql.org ([2001:4800:1501:1::229]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1cQAEQ-0001fJ-JQ for pgadmin-hackers@postgresql.org; Sun, 08 Jan 2017 09:57:42 +0000 Received: from mail-it0-x229.google.com ([2607:f8b0:4001:c0b::229]) by makus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.84_2) (envelope-from ) id 1cQAEN-0001bl-Bt for pgadmin-hackers@postgresql.org; Sun, 08 Jan 2017 09:57:41 +0000 Received: by mail-it0-x229.google.com with SMTP id r185so5354690ita.0 for ; Sun, 08 Jan 2017 01:57:39 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pgadmin-org.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=HdxsB/kL/kPedQkYtqKtX/5cFTGHn8jgNXAOP5eYSfs=; b=tTkIL2SK8EvcFG/MhxyFQQSAGqLhVXeknR3L3E0BokUraV56dIgv1149tnMAeDM+Gt gp+ZyMt3IoggfuvgKYWDLhOAaIIoh8ic18R/8ce1FxnP4yOO3a7Q+tJ6GEJNSgwCrxZ/ vBGksQ8oGw4ReYGx2Ax7GBpuUAheCaowQUcEkc0JgFyMCXa3LttoSv1J/nz0ERVYMsPl +Akvc8FwNDDF2T2ZKT1+DCGbkk5Qk+2qEUR+pwkbWcEuRHN//Usrh3DcS3dOWh9maRFd IlASSwWyDCcKYJy6xtrZUGuxAM3molbo57Pqjx93esGbfKtjFRKkkpvdlRE5LD99tP2Y UJXQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=HdxsB/kL/kPedQkYtqKtX/5cFTGHn8jgNXAOP5eYSfs=; b=nkto3G3th4pbHwGhKxXl85alDUaFuhISXzEee+joFVlo4tzZOMwNO4zYc73UJQEMfr bmRxlxCfQ30I+pyYmygqu47vOYzHGZTlMgC7895YyMnr+JMVp6w/orhzle7O4rKbeEer GveltmJ5z3+3YS+EW5mhGmkfD5e/BkMMcr3Ym8NNKnFd+eEf6HL9+nOdVKqsZAbW5Lva WibQGdZN7DuF3G4l3oFmnO+KK122LoF+oTs1UdRiUuijDdd5NUGlR/lf8wrSLhfu61va l6Fmw/1yrvbwmplEqBynJ/HKim7APTH0Hp5D1e39Vgeehpg5EKBdqW8ed+3DpLGMIfjY E/vQ== X-Gm-Message-State: AIkVDXJUvatK3F/QE2RuvUxoJfu4VJeLopbr0+WTEqLkV9jipPXJ1lhACtgX+Sls3EyFZ5e4mkMLpWePjomXYg== X-Received: by 10.36.206.134 with SMTP id v128mr5095078itg.94.1483869458717; Sun, 08 Jan 2017 01:57:38 -0800 (PST) MIME-Version: 1.0 Received: by 10.64.224.198 with HTTP; Sun, 8 Jan 2017 01:57:37 -0800 (PST) In-Reply-To: References: From: Dave Page Date: Sun, 8 Jan 2017 09:57:37 +0000 Message-ID: Subject: Re: [pgAdmin4][Patch]: RM #1807 Query Tool Does Not Recognize When File Changes Have Been Saved To: Akshay Joshi Cc: pgadmin-hackers Content-Type: multipart/alternative; boundary=94eb2c0af818fae3c40545924745 X-Pg-Spam-Score: -2.6 (--) List-Archive: List-Help: List-ID: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: X-Mailing-List: pgadmin-hackers Precedence: bulk Sender: pgadmin-hackers-owner@postgresql.org --94eb2c0af818fae3c40545924745 Content-Type: text/plain; charset=UTF-8 Thanks, patch applied. On Friday, December 23, 2016, Akshay Joshi wrote: > Hi Dave > > I have fixed all the comments given by you. Attached is the modified patch > file. Please review it. > > On Fri, Dec 16, 2016 at 7:08 PM, Dave Page > wrote: > >> >> >> On Fri, Dec 16, 2016 at 1:24 PM, Akshay Joshi < >> akshay.joshi@enterprisedb.com >> > wrote: >> >>> >>> >>> On Fri, Dec 16, 2016 at 6:28 PM, Dave Page >> > wrote: >>> >>>> Hi >>>> >>>> On Fri, Dec 16, 2016 at 12:46 PM, Akshay Joshi < >>>> akshay.joshi@enterprisedb.com >>>> > wrote: >>>> >>>>> Hi Dave >>>>> >>>>> On Fri, Dec 16, 2016 at 5:22 PM, Dave Page >>>> > wrote: >>>>> >>>>>> Hi >>>>>> >>>>>> On Fri, Dec 16, 2016 at 8:47 AM, Akshay Joshi < >>>>>> akshay.joshi@enterprisedb.com >>>>>> > >>>>>> wrote: >>>>>> >>>>>>> Hi All >>>>>>> >>>>>>> Please find the attached patch to fix the RM #1807 Query Tool Does >>>>>>> Not Recognize When File Changes Have Been Saved. >>>>>>> >>>>>> >>>>>> If I open a file with the patch applied, and make a change (add a >>>>>> space to the end), it's correctly detected. >>>>>> >>>>>> If I then undo the change by deleting the space, the file is still >>>>>> marked as dirty. >>>>>> >>>>> >>>>> This was an old behaviour. To achieve this we need to compare the >>>>> original content with changed content on each key press event. >>>>> >>>> >>>> OK, but it's inconsistent with what happens if I'm not working with a >>>> file - in that case if I add a character and then remove it (so the query >>>> is empty again), then it does mark the editor as clean again. >>>> >>>> If you look at pgAdmin III, then any change to either an opened file, >>>> or a blank editor will mark it dirty. It never marks it as clean again. >>>> That means no content comparison, and consistent behaviour. >>>> >>> >>> With current implementation if we are not working with file then >>> there is no dirty(*) mark on the tab and if query is empty then no point in >>> saving/creating empty file that's why save button is disabled. Do you want >>> me to add an (*) mark there as well ? >>> >> >> Yes - and allow saving the empty file. Use pgAdmin III for reference - >> whilst there may be no point in saving an empty file, the behaviour is >> consistent and appropriate here. You don't run into a situation where you >> may have an empty editor with the ability to save one day, and without the >> next day. >> >> >>> >>>> >>>> >>>>> >>>>>> If I then clear the window entirely, the save button is disabled, but >>>>>> the tab still shows the file is dirty (the *). >>>>>> >>>>> >>>>> Again this was an old behaviour, I have added only one if condition >>>>> to check whether to popped up "Unsaved Changes" message or not. >>>>> >>>> >>>> Maybe - but the point is to fix the old behaviour :-). The dirty marker >>>> and enabled/disabled state of the save option should be in sync shouldn't >>>> they? I shouldn't expect to see a file marked as dirty that I cannot save >>>> (even if it is empty). >>>> >>> >>> OK. I'll enable the save button in that case for consistency. >>> >>>> >>>> >>>>> >>>>>> Also - the patch seems to undo the change I made >>>>>> in 4a280b251755091af9bf56bcdee964601df104ae. >>>>>> >>>>> >>>>> As per commit id 4a280b251755091af9bf56bcdee964601df104ae, you >>>>> have made following changes: >>>>> >>>>> - self.setTitle(self.gridView.current_file.re >>>>> place(/^\/|\/$/g, '')); >>>>> + self.setTitle(self.gridView.current_file.sp >>>>> lit('\\').pop().split('/').pop()); >>>>> >>>>> And as per me I haven't change any thing there, I have only disabled >>>>> the save button when file is loaded which was not there previously. >>>>> >>>> >>>> Ahh, no - I see the issue. When the file is marked as dirty, it uses >>>> the full path again. Can you fix that to use just the filename please? >>>> >>> >>> Sure. >>> >> >> Thanks. >> >> >>> >>>> Thanks. >>>> >>>> -- >>>> 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-9517 <+91%2020%203058%209517>Mobile: +91 >>> 976-788-8246* >>> >> >> >> >> -- >> 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* > -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company --94eb2c0af818fae3c40545924745 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Thanks, patch applied.

On Friday, December 23, 2016, Akshay Joshi &l= t;akshay.joshi@enterprised= b.com> wrote:
Hi = Dave

I have fixed all the comments given by you. Attache= d is the modified patch file. Please review it.=C2=A0

On Fri, Dec 16, 2016 at 7:08 PM, Da= ve Page <dpage@pgadmin.org>= ; wrote:


On Fri, Dec 16,= 2016 at 1:24 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:


On Fri, Dec 16, 2016 at 6:28 PM, Dave= Page <dpage@pgadmin.org><= /span> wrote:
Hi

On Fri, Dec 16,= 2016 at 12:46 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi Dave

On Fri, Dec 16, 2016 at 5:22 PM, D= ave Page <dpage@pgadmin.org&g= t; wrote:
Hi

On Fri, Dec 16, 2016 at 8:= 47 AM, Akshay Joshi <= akshay.joshi@enterprisedb.com> wrote:
Hi All

Please find the= attached patch to fix the RM=C2=A0#1807 Query Tool Does Not Recognize When= File Changes Have Been Saved.

If I open a file with the patch applied, and make a change (add a sp= ace to the end), it's correctly detected.=C2=A0

If I then undo the change by deleting the space, the file is still marked= as dirty.=C2=A0

=
=C2=A0 =C2=A0This was an old behaviour. To achieve this we need to com= pare the original content with changed content on each key press event.

OK, but it'= s inconsistent with what happens if I'm not working with a file - in th= at case if I add a character and then remove it (so the query is empty agai= n), then it does mark the editor as clean again.

I= f you look at pgAdmin III, then any change to either an opened file, or a b= lank editor will mark it dirty. It never marks it as clean again. That mean= s no content comparison, and consistent behaviour.
<= /blockquote>

=C2=A0 =C2=A0With current implementa= tion if we are not working with file then there is no dirty(*) mark on the = tab and if query is empty then no point in saving/creating empty file that&= #39;s why save button is disabled. Do you want me to add an (*) mark there = as well ?=C2=A0

<= div>Yes - and allow saving the empty file. Use pgAdmin III for reference - = whilst there may be no point in saving an empty file, the behaviour is cons= istent and appropriate here. You don't run into a situation where you m= ay have an empty editor with the ability to save one day, and without the n= ext day.
=C2=A0
<= blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;border-left:1px= #ccc solid;padding-left:1ex">
<= div class=3D"gmail_quote">
=C2=A0
=C2=A0

If I then clear th= e window entirely, the save button is disabled, but the tab still shows the= file is dirty (the *).

=
=C2=A0 =C2=A0Again this was an old behaviour, I have added only= one if condition to check whether to popped up "Unsaved Changes"= message or not.=C2=A0

<= /span>
Maybe - but the point is to fix the old behaviour :-). The dirty= marker and enabled/disabled state of the save option should be in sync sho= uldn't they? I shouldn't expect to see a file marked as dirty that = I cannot save (even if it is empty).

=C2=A0 =C2=A0 OK. I'll enable the save button = in that case for consistency. =C2=A0
=
=C2=A0
=

Also - the patch seems to undo the change I made in=C2=A04a= 280b251755091af9bf56bcdee964601df104ae.

=C2=A0 =C2=A0 As per commit id 4a280b25175= 5091af9bf56bcdee964601df104ae, you have made following changes:=C2=A0<= /div>

-=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0self.setTitle(self.= gridView.current_file.re<= wbr>place(/^\/|\/$/g,=C2=A0''));
+=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0self.setTitle(self.gridView.current_file.split('\\').pop().split('/').p= op());

And as per me I haven't change any = thing there, I have only disabled the save button when file is loaded which= was not there previously.

Ahh, no - I see the issue. When the file is marked = as dirty, it uses the full path again. Can you fix that to use just the fil= ename please?=C2=A0

=C2=A0 =C2=A0Sure.=C2=A0
<= br>
Thanks.
=C2=A0
=

Thanks.

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

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



--
Akshay Joshi
Principal So= ftware Engineer=C2=A0
<= b>

Phone: +91 20-3058-9517
Mobile: +91 976-788-8= 246



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

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



--
<= b>Akshay Joshi
Principal Software Enginee= r=C2=A0

=

http://pgsnake.blogspot.com
Twitter: @= pgsnake

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

--94eb2c0af818fae3c40545924745--