From: | Dave Page <dpage(at)pgadmin(dot)org> |
---|---|
To: | Matthew Kleiman <mkleiman(at)pivotal(dot)io> |
Cc: | pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>, George Gelashvili <ggelashvili(at)pivotal(dot)io> |
Subject: | Re: [patch] Column selection on SQLEditor |
Date: | 2017-04-10 15:13:21 |
Message-ID: | CA+OCxoz=7SnOrf2KP-+2c8o2uLANs1SCVd09jdq_Fp7AS3boNw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgadmin-hackers |
Hi
On Mon, Apr 10, 2017 at 4:01 PM, Matthew Kleiman <mkleiman(at)pivotal(dot)io> wrote:
> Hi Dave,
>
>> Unfortunately I found another regression; I have a test table called
>> موسيقى (Arabic for music) which if included in a selection of rows,
>
> Could you share the queries that:
> - create this table
CREATE TABLE public."موسيقى"
(
)
> - populate it with data
It doesn't have any - there are no columns in it.
> - access it leading to the regression
SELECT * FROM pg_tables
or
SELECT * FROM pg_class
in the Query Tool.
> What user interaction are you performing to trigger the behavior (is it the
> query tool or something else)?
Run the select query in the query tool, select the rows, then hit Copy.
Note that I think the issue with columns is the editor I was using. If
I paste into vim, I can't reproduce that. The row issue is
consistently reproducible though.
> It would also help if you could attach a screenshot of the issue.
Attached.
>
> Thanks,
> George & Matt
>
>
>
> On Mon, Apr 10, 2017 at 5:52 AM Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>
>> Hi Matt,
>>
>> On Fri, Apr 7, 2017 at 10:56 PM, Matthew Kleiman <mkleiman(at)pivotal(dot)io>
>> wrote:
>> > Hi Dave,
>> >
>> > I've updated the attached patch to include a change to the "Paste Rows"
>> > button. It will now be enabled only if there are rows on the clipboard.
>>
>> Unfortunately I found another regression; I have a test table called
>> موسيقى (Arabic for music) which if included in a selection of rows,
>> seems to ensure that it and any preceding rows are excluded from the
>> copy. If I include it in a selection of columns, then things get weird
>> as the pasted output for that row will sometimes - but not always -
>> seem to have the column order moved around (maybe because it's an RTL
>> language).
>>
>> Obviously the second issue doesn't apply to the current code, but
>> copying rows does seem to work properly at present, so I'm afraid I
>> need to bump this back to you again.
>>
>> > Good luck with the release on Monday morning!
>>
>> Thanks!
>>
>> > On Fri, Apr 7, 2017 at 4:12 PM, Matthew Kleiman <mkleiman(at)pivotal(dot)io>
>> > wrote:
>> >>
>> >> Hi Dave
>> >>
>> >> The attached patch now includes a fix for the regression you found.
>> >> When
>> >> the query tool is in edit mode, the user can copy rows and paste them.
>> >> I've
>> >> also removed the two lines from copy_data.js that you mentioned so that
>> >> users can copy from the table even after they have copied once.
>> >>
>> >> I have noticed that the "Paste Rows" button remains enabled even if
>> >> only
>> >> columns are copied. Although pressing this button won't do anything
>> >> unless
>> >> rows have been copied to the clipboard, this might prove confusing to
>> >> users.
>> >> I am going to look at disabling this button until rows are in the
>> >> clipboard.
>> >> I will let you know if I have anything for you by the end of today.
>> >>
>> >> Best,
>> >> Matt
>> >>
>> >> On Fri, Apr 7, 2017 at 10:27 AM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>> >>>
>> >>> On Fri, Apr 7, 2017 at 2:49 PM, Atira Odhner <aodhner(at)pivotal(dot)io>
>> >>> wrote:
>> >>> >> The one tweak I made to the patch was to remove the code that
>> >>> >> disabled
>> >>> >> the Copy button from the top of copy_data.js. I think the button
>> >>> >> should remain enabled to allow the user to copy again, in case they
>> >>> >> use the clipboard for something else and then need to refresh it
>> >>> >> with
>> >>> >> the data. Of course, it should still be disabled when there is
>> >>> >> nothing
>> >>> >> selected that can be copied.
>> >>> >
>> >>> > Yes, the copy button enablement behavior was a bit strange. I'm
>> >>> > glad
>> >>> > you
>> >>> > found a fix for it. Do you mind sending us your updated patch?
>> >>>
>> >>> I literally just removed lines 5 & 6 of copy_data.js.
>> >>>
>> >>> >> Any chance this can be fixed before Monday? I would like to include
>> >>> >> it
>> >>> >> in the release if possible
>> >>> >
>> >>> > I'll drop a bug at the top of our backlog, and Matt will take a look
>> >>> > at
>> >>> > it
>> >>> > today. We'll let you know at the end of the day where we're at with
>> >>> > this
>> >>> > fix.
>> >>>
>> >>> Thanks - and sorry to hear your moving onto other things :-(
>> >>>
>> >>> --
>> >>> Dave Page
>> >>> Blog: http://pgsnake.blogspot.com
>> >>> Twitter: @pgsnake
>> >>>
>> >>> EnterpriseDB UK: http://www.enterprisedb.com
>> >>> The Enterprise PostgreSQL Company
>> >>
>> >>
>> >
>>
>>
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EnterpriseDB UK: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Attachment | Content-Type | Size |
---|---|---|
Screen Shot 2017-04-10 at 16.08.10.png | image/png | 228.0 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Surinder Kumar | 2017-04-11 09:51:58 | [pgAdmin4][Patch]: RM#2333 - Server Activity data is not updating when server is disconnected in Dashboards |
Previous Message | Matthew Kleiman | 2017-04-10 15:01:56 | Re: [patch] Column selection on SQLEditor |