Re: [pgAdmin4][Patch]: RM #1807 Query Tool Does Not Recognize When File Changes Have Been Saved

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 <dpage(at)pgadmin(dot)org> wrote:

>
>
> On Fri, Dec 16, 2016 at 1:24 PM, Akshay Joshi <
> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>
>>
>>
>> On Fri, Dec 16, 2016 at 6:28 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>
>>> Hi
>>>
>>> On Fri, Dec 16, 2016 at 12:46 PM, Akshay Joshi <
>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>
>>>> Hi Dave
>>>>
>>>> On Fri, Dec 16, 2016 at 5:22 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>>
>>>>> Hi
>>>>>
>>>>> On Fri, Dec 16, 2016 at 8:47 AM, Akshay Joshi <
>>>>> akshay(dot)joshi(at)enterprisedb(dot)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*