public inbox for [email protected]
help / color / mirror / Atom feedFrom: Akshay Joshi <[email protected]>
To: Dave Page <[email protected]>
Cc: pgadmin-hackers <[email protected]>
Subject: Re: [pgAdmin4][Patch]: RM #1807 Query Tool Does Not Recognize When File Changes Have Been Saved
Date: Fri, 23 Dec 2016 16:22:40 +0530
Message-ID: <CANxoLDcPC+x_sZyBE7DJ82_Y5cSZagew_PXz67mck4Xfgqcoig@mail.gmail.com> (raw)
In-Reply-To: <CA+OCxozifgntS4xGjK74+_cPMSouMRfL4km4OPx4q8z5Du7orQ@mail.gmail.com>
References: <CANxoLDeuyz5d1+kKmpXiSdDMmtRGnKV+6dNmvtHeBTPjw6XV3w@mail.gmail.com>
<CA+OCxoye7d0F8dt8UKAQan2b+fzVB=v919=MTkwVZ4R-wx3Hug@mail.gmail.com>
<CANxoLDcPAPga6nuhMUZ-cYrcsidmko7_OP+kVfgPVRmK-dpxiA@mail.gmail.com>
<CA+OCxoyo9RTn=5R7mTueq65sk=TN49DYucGAb2FdYj3qvx5tdA@mail.gmail.com>
<CANxoLDektZovbGmgEgh7+4dUaCgKHq9txDg8k=272c2xLanqRg@mail.gmail.com>
<CA+OCxozifgntS4xGjK74+_cPMSouMRfL4km4OPx4q8z5Du7orQ@mail.gmail.com>
List-Unsubscribe: <mailto:[email protected]?body=unsub%20pgadmin-hackers>
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 <[email protected]> wrote:
>
>
> On Fri, Dec 16, 2016 at 1:24 PM, Akshay Joshi <
> [email protected]> wrote:
>
>>
>>
>> On Fri, Dec 16, 2016 at 6:28 PM, Dave Page <[email protected]> wrote:
>>
>>> Hi
>>>
>>> On Fri, Dec 16, 2016 at 12:46 PM, Akshay Joshi <
>>> [email protected]> wrote:
>>>
>>>> Hi Dave
>>>>
>>>> On Fri, Dec 16, 2016 at 5:22 PM, Dave Page <[email protected]> wrote:
>>>>
>>>>> Hi
>>>>>
>>>>> On Fri, Dec 16, 2016 at 8:47 AM, Akshay Joshi <
>>>>> [email protected]> 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*
--
Sent via pgadmin-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers
Attachments:
[application/octet-stream] RM_1807_v2.patch (7.4K, 3-RM_1807_v2.patch)
download | inline diff:
diff --git a/web/pgadmin/tools/sqleditor/templates/sqleditor/js/sqleditor.js b/web/pgadmin/tools/sqleditor/templates/sqleditor/js/sqleditor.js
index 698e84d..5e400c7 100644
--- a/web/pgadmin/tools/sqleditor/templates/sqleditor/js/sqleditor.js
+++ b/web/pgadmin/tools/sqleditor/templates/sqleditor/js/sqleditor.js
@@ -307,15 +307,9 @@ define(
msg = '{{ _('The data has been modified, but not saved. Are you sure you wish to discard the changes?') }}';
notify = true;
}
- } else if(self.handler.is_query_tool) {
- // We will check for modified sql content
- var sql = self.handler.gridView.query_tool_obj.getValue();
- sql = sql.replace(/\s+/g, '');
- // If it is an empty query, do nothing.
- if (sql.length > 0) {
- msg = '{{ _('The query has been modified, but not saved. Are you sure you wish to discard the changes?') }}';
- notify = true;
- }
+ } else if(self.handler.is_query_tool && self.handler.is_query_changed) {
+ msg = '{{ _('The query has been modified, but not saved. Are you sure you wish to discard the changes?') }}';
+ notify = true;
}
if(notify) {return self.user_confirmation(p, msg);}
return true;
@@ -1330,23 +1324,24 @@ define(
this._stopEventPropogation(ev);
this._closeDropDown(ev);
- // We will check for modified sql content
- sql = self.query_tool_obj.getValue();
- sql = sql.replace(/\s+/g, '');
- // If there is nothing to save, clear it.
- if (!sql.length) { self.query_tool_obj.setValue(''); return; }
-
- alertify.confirm(
- '{{ _('Unsaved changes') }}',
- '{{ _('Are you sure you wish to discard the current changes?') }}',
- function() {
- // Do nothing as user do not want to save, just continue
- self.query_tool_obj.setValue('');
- },
- function() {
- return true;
- }
- ).set('labels', {ok:'Yes', cancel:'No'});
+ /* If is_query_changed flag is set to false then no need to
+ * confirm with the user for unsaved changes.
+ */
+ if (self.handler.is_query_changed) {
+ alertify.confirm(
+ '{{ _('Unsaved changes') }}',
+ '{{ _('Are you sure you wish to discard the current changes?') }}',
+ function() {
+ // Do nothing as user do not want to save, just continue
+ self.query_tool_obj.setValue('');
+ },
+ function() {
+ return true;
+ }
+ ).set('labels', {ok:'Yes', cancel:'No'});
+ } else {
+ self.query_tool_obj.setValue('');
+ }
},
// Callback function for the clear history button click.
@@ -2246,7 +2241,6 @@ define(
// Open save file dialog if query tool
if (self.is_query_tool) {
-
var current_file = self.gridView.current_file;
if (!_.isUndefined(current_file) && !save_as) {
self._save_file_handler(current_file);
@@ -2428,22 +2422,25 @@ define(
// load select file dialog
_load_file: function() {
var self = this;
- // We will check for modified sql content
- sql = self.gridView.query_tool_obj.getValue()
- sql = sql.replace(/\s+/g, '');
- // If there is nothing to save, open file manager.
- if (!sql.length) { self._open_select_file_manager(); return; }
- alertify.confirm('{{ _('Unsaved changes') }}',
- '{{ _('Are you sure you wish to discard the current changes?') }}',
- function() {
- // User do not want to save, just continue
- self._open_select_file_manager();
- },
- function() {
- return true;
- }
- ).set('labels', {ok:'Yes', cancel:'No'});
+ /* If is_query_changed flag is set to false then no need to
+ * confirm with the user for unsaved changes.
+ */
+ if (self.is_query_changed) {
+ alertify.confirm('{{ _('Unsaved changes') }}',
+ '{{ _('Are you sure you wish to discard the current changes?') }}',
+ function() {
+ // User do not want to save, just continue
+ self._open_select_file_manager();
+ },
+ function() {
+ return true;
+ }
+ ).set('labels', {ok:'Yes', cancel:'No'});
+ } else {
+ self._open_select_file_manager();
+ }
+
},
// Open FileManager
@@ -2486,6 +2483,13 @@ define(
self.trigger('pgadmin-sqleditor:loading-icon:hide');
// hide cursor
$busy_icon_div.removeClass('show_progress');
+
+ // disable save button on file save
+ $("#btn-save").prop('disabled', true);
+ $("#btn-file-menu-save").css('display', 'none');
+
+ // Update the flag as new content is just loaded.
+ self.is_query_changed = false;
},
error: function(e) {
var errmsg = $.parseJSON(e.responseText).errormsg;
@@ -2519,10 +2523,13 @@ define(
if (res.data.status) {
alertify.success('{{ _('File saved successfully.') }}');
self.gridView.current_file = e;
- self.setTitle(self.gridView.current_file.replace(/^\/|\/$/g, ''));
+ self.setTitle(self.gridView.current_file.replace(/^.*[\\\/]/g, ''));
// disable save button on file save
$("#btn-save").prop('disabled', true);
$("#btn-file-menu-save").css('display', 'none');
+
+ // Update the flag as query is already saved.
+ self.is_query_changed = false;
}
self.trigger('pgadmin-sqleditor:loading-icon:hide');
},
@@ -2542,15 +2549,28 @@ define(
// codemirror text change event
_on_query_change: function(query_tool_obj) {
var self = this;
- if(query_tool_obj.getValue().length == 0) {
- $("#btn-save").prop('disabled', true);
- $("#btn-file-menu-save").css('display', 'none');
- $("#btn-file-menu-dropdown").prop('disabled', true);
- } else {
+
+ if (!self.is_query_changed) {
+ // Update the flag as query is going to changed.
+ self.is_query_changed = true;
+
if(self.gridView.current_file) {
- var title = self.gridView.current_file.replace(/^\/|\/$/g, '') + ' *'
+ var title = self.gridView.current_file.replace(/^.*[\\\/]/g, '') + ' *'
+ self.setTitle(title);
+ } else {
+ var title = '';
+
+ // Find the title of the visible panel
+ _.each(window.top.pgAdmin.Browser.docker.findPanels('frm_datagrid'), function(p) {
+ if(p.isVisible()) {
+ self.gridView.panel_title = p._title;
+ }
+ });
+
+ title = self.gridView.panel_title + ' *';
self.setTitle(title);
}
+
$("#btn-save").prop('disabled', false);
$("#btn-file-menu-save").css('display', 'block');
$("#btn-file-menu-dropdown").prop('disabled', false);
view thread (8+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected]
Subject: Re: [pgAdmin4][Patch]: RM #1807 Query Tool Does Not Recognize When File Changes Have Been Saved
In-Reply-To: <CANxoLDcPC+x_sZyBE7DJ82_Y5cSZagew_PXz67mck4Xfgqcoig@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox