Re: [pgadmin-hackers][patch] Column selection on SQLEditor

Hi

On Mon, Apr 10, 2017 at 4:01 PM, Matthew Kleiman <mkleiman@pivotal.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@pgadmin.org> wrote:
>>
>> Hi Matt,
>>
>> On Fri, Apr 7, 2017 at 10:56 PM, Matthew Kleiman <mkleiman@pivotal.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@pivotal.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@pgadmin.org> wrote:
>> >>>
>> >>> On Fri, Apr 7, 2017 at 2:49 PM, Atira Odhner <aodhner@pivotal.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