Re: [pgadmin-hackers] [pgAdmin4][PATCH] Improvements to Query ResultsGrid User Experience

Поиск
Список
Период
Сортировка
От Matthew Kleiman
Тема Re: [pgadmin-hackers] [pgAdmin4][PATCH] Improvements to Query ResultsGrid User Experience
Дата
Msg-id CAFS4TJbJ9+0SyKOXjWKii8bw+p4WBUYqcwQAzrXySXig4NU-zg@mail.gmail.com
обсуждение исходный текст
Ответ на Re: [pgadmin-hackers] [pgAdmin4][PATCH] Improvements to Query ResultsGrid User Experience  (Robert Eckhardt <reckhardt@pivotal.io>)
Ответы Re: [pgadmin-hackers] [pgAdmin4][PATCH] Improvements to Query Results Grid User Experience  (Surinder Kumar <surinder.kumar@enterprisedb.com>)
Список pgadmin-hackers
Hi Dave!

It looks like everything has been addressed with this current patch, aside from the feature test for for edit table tool. If we agree that Surinder can send that patch on a separate thread, is this patch good to be committed?

Thanks,
Matt

On Thu, Jun 1, 2017 at 11:17 AM, Robert Eckhardt <reckhardt@pivotal.io> wrote:
Awesome, thanks.

-- Rob

On Thu, Jun 1, 2017 at 11:13 AM, Surinder Kumar <surinder.kumar@enterprisedb.com> wrote:

Hi Robert,

On Jun 1, 2017 8:22 PM, "Robert Eckhardt" <reckhardt@pivotal.io> wrote:
>
> Surindar, 
>
> Have you sent this patch and I'm missing it or is it still in flight for you? 
I have found the solution, will test that patch and hopefully by tomorrow I will send.


>
> -- Rob
>
> On Wed, May 31, 2017 at 1:02 AM, Surinder Kumar <surinder.kumar@enterprisedb.com> wrote:
>>
>> Hi Joao
>>
>> On Wed, May 31, 2017 at 1:19 AM, Joao Pedro De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
>>>
>>> Hello
>>>
>>> We have rebased the patches against master again, which includes Surinder's fix for RM2400. These patches should now apply against the HEAD of master.
>>>>
>>>>
>>>> Given these issues, I think it would be sensible to add a feature test
>>>> to copy/paste a couple of existing rows in a table, blank out the pkey
>>>> values, save then refresh, and check everything looks right. Thoughts?
>>>
>>> Currently, the feature test, CopySelectedQueryResultsFeatureTest, only covers copy functionality in the query tool. It sounds like we could use some additional coverage around the Edit Table tool, which could include paste rows functionality. Can we get this patch merged and create a RedMine issue that enumerates the additional functionality we want to cover via feature test? 
>>
>> ​FeatureTest for edit Table tool and copy/paste rows are already written, but there is some flakiness when executing. I will be sending that patch again with fix.​
>>>
>>>
>>> Thanks,
>>> Joao & Matt
>>>
>>>
>>> On Sat, May 27, 2017 at 2:19 PM, Dave Page <dpage@pgadmin.org> wrote:
>>>>
>>>> On Sat, May 27, 2017 at 9:02 AM, Surinder Kumar
>>>> <surinder.kumar@enterprisedb.com> wrote:
>>>> > Hi Dave,
>>>> > On Sat, May 27, 2017 at 3:07 AM, Dave Page <dpage@pgadmin.org> wrote:
>>>> >>
>>>> >> Hi
>>>> >>
>>>> >> OK, so we're getting somewhere now :-). Here's what I found:
>>>> >>
>>>> >> - The grid looks and feels great now. Selection works nicely, and
>>>> >> copying rows seems to work well.
>>>> >>
>>>> >> - Patch 5 doesn't apply - but only to slick.grid.js. I manually fixed
>>>> >> it for testing, but the patch really doesn't look like it was created
>>>> >> from the version of the file from GIT HEAD with patches 1 - 4 applied.
>>>> >>
>>>> >> - There is a problem. I noticed that a) when copy/pasting rows, if I
>>>> >> blank out a key column with a default it gets set to [null] instead of
>>>> >> [default] and b) inserting rows seems to get the values of the new
>>>> >> row(s) from the wrong place, resulting in duplicate key violations.
>>>> >
>>>> > This belongs to RM2400 - handle [default] and [null] while copy/pasting
>>>> > rows. An updated patch and Feature tests are already sent.
>>>> > You can review and commit if looks good.
>>>>
>>>> That part, yes - but not mixing up the values.
>>>>
>>>>
>>>> >> Now, both of these issues sound very much like ones Surinder fixed
>>>> >> recently following other improvements to the grid to more properly
>>>> >> handle nulls and default values. I *think* these fixes were in commit:
>>>> >> d7d4bf475bc5b131d9a76376ebfc87e004d92333. Perhaps there's been a
>>>> >> merging error in your development branch at some point?
>>>> >>
>>>> >> Given these issues, I think it would be sensible to add a feature test
>>>> >> to copy/paste a couple of existing rows in a table, blank out the pkey
>>>> >> values, save then refresh, and check everything looks right. Thoughts?
>>>> >>
>>>> >> I'm looking forward to seeing this fully baked :-)
>>>> >>
>>>> >> Thanks!
>>>> >>
>>>> >> On Fri, May 26, 2017 at 11:23 AM, Joao Pedro De Almeida Pereira
>>>> >> <jdealmeidapereira@pivotal.io> wrote:
>>>> >> > Hello Dave,
>>>> >> >
>>>> >> > We created the previous patches using the command recommended in the
>>>> >> > pgAdmin
>>>> >> > website, but apparently the diff doesn't work correctly when you have
>>>> >> > binary
>>>> >> > files in the commit, like images.
>>>> >> >
>>>> >> > We regenerated the patches using the command with the addition of
>>>> >> > --binary
>>>> >> >
>>>> >> > git diff e0d5cf6b ac65f43b --binary > 6-adapt-slickgrid-to-pgadmin.patch
>>>> >> >
>>>> >> > After that, we applied them in master and this was the output:
>>>> >> >
>>>> >> > ± jp+si |master → origin {9} ✓| → git apply 1-upgrade-slickgrid.patch
>>>> >> > 1-upgrade-slickgrid.patch:120: trailing whitespace.
>>>> >> >
>>>> >> > 1-upgrade-slickgrid.patch:129: trailing whitespace.
>>>> >> >
>>>> >> > 1-upgrade-slickgrid.patch:138: trailing whitespace.
>>>> >> >
>>>> >> > 1-upgrade-slickgrid.patch:147: trailing whitespace.
>>>> >> >
>>>> >> > 1-upgrade-slickgrid.patch:156: trailing whitespace.
>>>> >> >
>>>> >> > warning: squelched 3856 whitespace errors
>>>> >> > warning: 3861 lines add whitespace errors.
>>>> >> >
>>>> >> >   maujer in ~/workspace/pgadmin4
>>>> >> > ± jp+si |master → origin {9} U:17 ?:7 ✗| → git apply
>>>> >> > 2-select-cells-improvements.patch
>>>> >> >
>>>> >> >   maujer in ~/workspace/pgadmin4
>>>> >> > ± jp+si |master → origin {9} U:35 ?:12 ✗| → git apply
>>>> >> > 3-drag-and-select.patch
>>>> >> >
>>>> >> >   maujer in ~/workspace/pgadmin4
>>>> >> > ± jp+si |master → origin {9} U:41 ?:13 ✗| → git apply
>>>> >> > 4-remove-checkboxes.patch
>>>> >> >
>>>> >> >   maujer in ~/workspace/pgadmin4
>>>> >> > ± jp+si |master → origin {9} U:41 ?:14 ✗| → git apply
>>>> >> > 5-improvements-to-range-selection.patch
>>>> >> > 5-improvements-to-range-selection.patch:647: trailing whitespace.
>>>> >> >     function scrollColumnIntoView(columnIndex) {
>>>> >> > 5-improvements-to-range-selection.patch:648: trailing whitespace.
>>>> >> >       var colspan = getColspan(row, columnIndex);
>>>> >> > 5-improvements-to-range-selection.patch:649: trailing whitespace.
>>>> >> >
>>>> >> > 5-improvements-to-range-selection.patch:650: trailing whitespace.
>>>> >> >       var left = columnPosLeft[columnIndex],
>>>> >> > 5-improvements-to-range-selection.patch:651: trailing whitespace.
>>>> >> >         right = columnPosRight[columnIndex + (colspan > 1 ? colspan - 1
>>>> >> > :
>>>> >> > 0)],
>>>> >> > warning: squelched 15 whitespace errors
>>>> >> > warning: 20 lines add whitespace errors.
>>>> >> >
>>>> >> >   maujer in ~/workspace/pgadmin4
>>>> >> > ± jp+si |master → origin {9} U:41 ?:16 ✗| → git apply
>>>> >> > 6-adapt-slickgrid-to-pgadmin.patch
>>>> >> >
>>>> >> >
>>>> >> > Maybe we should update the website with this information to help future
>>>> >> > committers.
>>>> >> >
>>>> >> > Thanks
>>>> >> > Joao & Shruti
>>>> >> >
>>>> >> >
>>>> >> > On Fri, May 26, 2017 at 10:35 AM, Dave Page <dpage@pgadmin.org> wrote:
>>>> >> >>
>>>> >> >> Hi,
>>>> >> >>
>>>> >> >> These patches really don't look like they were created in the normal
>>>> >> >> way:
>>>> >> >>
>>>> >> >> (pgadmin4)snake:web dpage$ git apply
>>>> >> >> ~/Downloads/1-upgrade-slickgrid.patch
>>>> >> >> /Users/dpage/Downloads/1-upgrade-slickgrid.patch:126: trailing
>>>> >> >> whitespace.
>>>> >> >>
>>>> >> >> /Users/dpage/Downloads/1-upgrade-slickgrid.patch:129: trailing
>>>> >> >> whitespace.
>>>> >> >>       where the browser copies/pastes the serialized data.
>>>> >> >> /Users/dpage/Downloads/1-upgrade-slickgrid.patch:130: trailing
>>>> >> >> whitespace.
>>>> >> >>
>>>> >> >> /Users/dpage/Downloads/1-upgrade-slickgrid.patch:154: trailing
>>>> >> >> whitespace.
>>>> >> >>
>>>> >> >> /Users/dpage/Downloads/1-upgrade-slickgrid.patch:165: trailing
>>>> >> >> whitespace.
>>>> >> >>
>>>> >> >> error: cannot apply binary patch to
>>>> >> >> 'web/pgadmin/static/vendor/slickgrid/images/CheckboxN.png' without
>>>> >> >> full index line
>>>> >> >> error: web/pgadmin/static/vendor/slickgrid/images/CheckboxN.png: patch
>>>> >> >> does not apply
>>>> >> >> error: cannot apply binary patch to
>>>> >> >> 'web/pgadmin/static/vendor/slickgrid/images/CheckboxY.png' without
>>>> >> >> full index line
>>>> >> >> error: web/pgadmin/static/vendor/slickgrid/images/CheckboxY.png: patch
>>>> >> >> does not apply
>>>> >> >> error: patch failed:
>>>> >> >>
>>>> >> >> web/pgadmin/static/vendor/slickgrid/plugins/slick.cellselectionmodel.js:94
>>>> >> >> error:
>>>> >> >>
>>>> >> >> web/pgadmin/static/vendor/slickgrid/plugins/slick.cellselectionmodel.js:
>>>> >> >> patch does not apply
>>>> >> >> error: patch failed:
>>>> >> >> web/pgadmin/static/vendor/slickgrid/slick.grid.css:102
>>>> >> >> error: web/pgadmin/static/vendor/slickgrid/slick.grid.css: patch does
>>>> >> >> not
>>>> >> >> apply
>>>> >> >> error: patch failed:
>>>> >> >> web/pgadmin/static/vendor/slickgrid/slick.grid.js:1
>>>> >> >> error: web/pgadmin/static/vendor/slickgrid/slick.grid.js: patch does
>>>> >> >> not
>>>> >> >> apply
>>>> >> >> (pgadmin4)snake:web dpage$ cd ../
>>>> >> >> (pgadmin4)snake:pgadmin4 dpage$ git apply
>>>> >> >> ~/Downloads/1-upgrade-slickgrid.patch
>>>> >> >> /Users/dpage/Downloads/1-upgrade-slickgrid.patch:126: trailing
>>>> >> >> whitespace.
>>>> >> >>
>>>> >> >> /Users/dpage/Downloads/1-upgrade-slickgrid.patch:129: trailing
>>>> >> >> whitespace.
>>>> >> >>       where the browser copies/pastes the serialized data.
>>>> >> >> /Users/dpage/Downloads/1-upgrade-slickgrid.patch:130: trailing
>>>> >> >> whitespace.
>>>> >> >>
>>>> >> >> /Users/dpage/Downloads/1-upgrade-slickgrid.patch:154: trailing
>>>> >> >> whitespace.
>>>> >> >>
>>>> >> >> /Users/dpage/Downloads/1-upgrade-slickgrid.patch:165: trailing
>>>> >> >> whitespace.
>>>> >> >>
>>>> >> >> error: cannot apply binary patch to
>>>> >> >> 'web/pgadmin/static/vendor/slickgrid/images/CheckboxN.png' without
>>>> >> >> full index line
>>>> >> >> error: web/pgadmin/static/vendor/slickgrid/images/CheckboxN.png: patch
>>>> >> >> does not apply
>>>> >> >> error: cannot apply binary patch to
>>>> >> >> 'web/pgadmin/static/vendor/slickgrid/images/CheckboxY.png' without
>>>> >> >> full index line
>>>> >> >> error: web/pgadmin/static/vendor/slickgrid/images/CheckboxY.png: patch
>>>> >> >> does not apply
>>>> >> >> error: patch failed:
>>>> >> >>
>>>> >> >> web/pgadmin/static/vendor/slickgrid/plugins/slick.cellselectionmodel.js:94
>>>> >> >> error:
>>>> >> >>
>>>> >> >> web/pgadmin/static/vendor/slickgrid/plugins/slick.cellselectionmodel.js:
>>>> >> >> patch does not apply
>>>> >> >> error: patch failed:
>>>> >> >> web/pgadmin/static/vendor/slickgrid/slick.grid.css:102
>>>> >> >> error: web/pgadmin/static/vendor/slickgrid/slick.grid.css: patch does
>>>> >> >> not
>>>> >> >> apply
>>>> >> >> error: patch failed:
>>>> >> >> web/pgadmin/static/vendor/slickgrid/slick.grid.js:1
>>>> >> >> error: web/pgadmin/static/vendor/slickgrid/slick.grid.js: patch does
>>>> >> >> not
>>>> >> >> apply
>>>> >> >>
>>>> >> >> I even tried with patch:
>>>> >> >>
>>>> >> >> (pgadmin4)snake:pgadmin4 dpage$ patch -p1 <
>>>> >> >> ~/Downloads/1-upgrade-slickgrid.patch
>>>> >> >> patching file libraries.txt
>>>> >> >> patching file web/pgadmin/static/vendor/slickgrid/README
>>>> >> >> patching file web/pgadmin/static/vendor/slickgrid/README.md
>>>> >> >> patching file
>>>> >> >> web/pgadmin/static/vendor/slickgrid/controls/slick.columnpicker.js
>>>> >> >> patching file
>>>> >> >> web/pgadmin/static/vendor/slickgrid/plugins/slick.cellcopymanager.js
>>>> >> >> patching file
>>>> >> >>
>>>> >> >> web/pgadmin/static/vendor/slickgrid/plugins/slick.cellexternalcopymanager.js
>>>> >> >> patching file
>>>> >> >> web/pgadmin/static/vendor/slickgrid/plugins/slick.cellrangeselector.js
>>>> >> >> patching file
>>>> >> >> web/pgadmin/static/vendor/slickgrid/plugins/slick.cellselectionmodel.js
>>>> >> >> Hunk #3 succeeded at 95 with fuzz 2.
>>>> >> >> patching file
>>>> >> >> web/pgadmin/static/vendor/slickgrid/plugins/slick.headerbuttons.js
>>>> >> >> patching file
>>>> >> >> web/pgadmin/static/vendor/slickgrid/plugins/slick.headermenu.js
>>>> >> >> patching file
>>>> >> >> web/pgadmin/static/vendor/slickgrid/plugins/slick.rowselectionmodel.js
>>>> >> >> patching file
>>>> >> >> web/pgadmin/static/vendor/slickgrid/slick-default-theme.css
>>>> >> >> patching file web/pgadmin/static/vendor/slickgrid/slick.core.js
>>>> >> >> patching file web/pgadmin/static/vendor/slickgrid/slick.dataview.js
>>>> >> >> patching file web/pgadmin/static/vendor/slickgrid/slick.editors.js
>>>> >> >> patching file web/pgadmin/static/vendor/slickgrid/slick.formatters.js
>>>> >> >> patching file web/pgadmin/static/vendor/slickgrid/slick.grid.css
>>>> >> >> Hunk #6 FAILED at 117.
>>>> >> >> 1 out of 9 hunks FAILED -- saving rejects to file
>>>> >> >> web/pgadmin/static/vendor/slickgrid/slick.grid.css.rej
>>>> >> >> patching file web/pgadmin/static/vendor/slickgrid/slick.grid.js
>>>> >> >> Hunk #1 FAILED at 1.
>>>> >> >> Hunk #2 FAILED at 18.
>>>> >> >> Hunk #3 FAILED at 75.
>>>> >> >> Hunk #4 FAILED at 89.
>>>> >> >> Hunk #5 FAILED at 129.
>>>> >> >> Hunk #6 FAILED at 143.
>>>> >> >> Hunk #7 FAILED at 174.
>>>> >> >> Hunk #8 FAILED at 208.
>>>> >> >> Hunk #9 FAILED at 289.
>>>> >> >> Hunk #10 FAILED at 330.
>>>> >> >> Hunk #11 FAILED at 343.
>>>> >> >> Hunk #12 FAILED at 385.
>>>> >> >> Hunk #13 FAILED at 457.
>>>> >> >> Hunk #14 FAILED at 491.
>>>> >> >> Hunk #15 FAILED at 510.
>>>> >> >> Hunk #16 FAILED at 548.
>>>> >> >> Hunk #17 FAILED at 557.
>>>> >> >> Hunk #18 FAILED at 600.
>>>> >> >> Hunk #19 FAILED at 652.
>>>> >> >> Hunk #20 FAILED at 686.
>>>> >> >> Hunk #21 FAILED at 706.
>>>> >> >> Hunk #22 FAILED at 749.
>>>> >> >> Hunk #23 FAILED at 858.
>>>> >> >> Hunk #24 FAILED at 870.
>>>> >> >> Hunk #25 FAILED at 886.
>>>> >> >> Hunk #26 FAILED at 937.
>>>> >> >> Hunk #27 FAILED at 957.
>>>> >> >> Hunk #28 FAILED at 987.
>>>> >> >> Hunk #29 FAILED at 1007.
>>>> >> >> Hunk #30 FAILED at 1039.
>>>> >> >> Hunk #31 FAILED at 1057.
>>>> >> >> Hunk #32 FAILED at 1080.
>>>> >> >> Hunk #33 FAILED at 1098.
>>>> >> >> Hunk #34 FAILED at 1117.
>>>> >> >> Hunk #35 FAILED at 1155.
>>>> >> >> Hunk #36 FAILED at 1267.
>>>> >> >> Hunk #37 FAILED at 1303.
>>>> >> >> Hunk #38 FAILED at 1317.
>>>> >> >> Hunk #39 FAILED at 1400.
>>>> >> >> Hunk #40 FAILED at 1415.
>>>> >> >> Hunk #41 FAILED at 1446.
>>>> >> >> Hunk #42 FAILED at 1485.
>>>> >> >> Hunk #43 FAILED at 1583.
>>>> >> >> Hunk #44 FAILED at 1600.
>>>> >> >> Hunk #45 FAILED at 1637.
>>>> >> >> Hunk #46 FAILED at 1650.
>>>> >> >> Hunk #47 FAILED at 1763.
>>>> >> >> Hunk #48 FAILED at 1780.
>>>> >> >> Hunk #49 FAILED at 1804.
>>>> >> >> Hunk #50 FAILED at 1818.
>>>> >> >> Hunk #51 FAILED at 1832.
>>>> >> >> Hunk #52 FAILED at 1848.
>>>> >> >> Hunk #53 FAILED at 1877.
>>>> >> >> Hunk #54 FAILED at 1893.
>>>> >> >> Hunk #55 FAILED at 2256.
>>>> >> >> Hunk #56 FAILED at 2274.
>>>> >> >> Hunk #57 FAILED at 2415.
>>>> >> >> Hunk #58 FAILED at 2474.
>>>> >> >> Hunk #59 FAILED at 2538.
>>>> >> >> Hunk #60 FAILED at 2606.
>>>> >> >> Hunk #61 FAILED at 2621.
>>>> >> >> Hunk #62 FAILED at 2724.
>>>> >> >> Hunk #63 FAILED at 2740.
>>>> >> >> Hunk #64 FAILED at 2798.
>>>> >> >> Hunk #65 FAILED at 2815.
>>>> >> >> Hunk #66 FAILED at 2839.
>>>> >> >> Hunk #67 FAILED at 2913.
>>>> >> >> Hunk #68 FAILED at 2928.
>>>> >> >> Hunk #69 FAILED at 2954.
>>>> >> >> Hunk #70 FAILED at 2974.
>>>> >> >> Hunk #71 FAILED at 3018.
>>>> >> >> Hunk #72 FAILED at 3307.
>>>> >> >> Hunk #73 FAILED at 3443.
>>>> >> >> Hunk #74 FAILED at 3454.
>>>> >> >> Hunk #75 FAILED at 3621.
>>>> >> >> Hunk #76 FAILED at 3662.
>>>> >> >> Hunk #77 FAILED at 3674.
>>>> >> >> Hunk #78 FAILED at 3684.
>>>> >> >> Hunk #79 FAILED at 3724.
>>>> >> >> Hunk #80 FAILED at 3770.
>>>> >> >> 80 out of 80 hunks FAILED -- saving rejects to file
>>>> >> >> web/pgadmin/static/vendor/slickgrid/slick.grid.js.rej
>>>> >> >> patching file
>>>> >> >> web/pgadmin/static/vendor/slickgrid/slick.groupitemmetadataprovider.js
>>>> >> >> patching file
>>>> >> >> web/pgadmin/static/vendor/slickgrid/slick.remotemodel-yahoo.js
>>>> >> >> patching file web/pgadmin/static/vendor/slickgrid/slick.remotemodel.js
>>>> >> >>
>>>> >> >> On Thu, May 25, 2017 at 2:35 PM, Shruti B Iyer <siyer@pivotal.io>
>>>> >> >> wrote:
>>>> >> >> > Hi
>>>> >> >> >
>>>> >> >> > Attached are the patches generated using diff and the correction of
>>>> >> >> > the
>>>> >> >> > bugs
>>>> >> >> > you mentioned in the previous email.
>>>> >> >> >
>>>> >> >> > We also split the initial first patch
>>>> >> >> > 0001-Improves-user-s-ability-to-select-cells-in-query-res.patch into
>>>> >> >> > two
>>>> >> >> > different patches because it included the upgrade of slickgrid and
>>>> >> >> > other
>>>> >> >> > changes making it hard to read. Now, the code review should be much
>>>> >> >> > easier
>>>> >> >> > and the upgrade of a vendor library becomes more clear in the commit
>>>> >> >> > history.
>>>> >> >> >
>>>> >> >> > The 6th patch 6-adapt-slickgrid-to-pgadmin.patch contains the edits
>>>> >> >> > to
>>>> >> >> > the
>>>> >> >> > slickgrid library that makes drag-and-select bounding box align with
>>>> >> >> > the
>>>> >> >> > gridlines. We created a PR to slickgrid that we are still discussing
>>>> >> >> > the
>>>> >> >> > implementation. Meanwhile. we created a temporary fix to solve the
>>>> >> >> > problem.
>>>> >> >> > We suggest that this 6th patch be committed separately.
>>>> >> >> >
>>>> >> >> > Thanks
>>>> >> >> > Joao & Shruti
>>>> >> >> >
>>>> >> >> > On Wed, May 24, 2017 at 8:18 AM, Dave Page <dpage@pgadmin.org> wrote:
>>>> >> >> >>
>>>> >> >> >> Hi
>>>> >> >> >>
>>>> >> >> >> We don't want this to be committed, even to a local tree (as there's
>>>> >> >> >> a
>>>> >> >> >> risk it may inadvertently get pushed). These patches are for review
>>>> >> >> >> only at this stage. Once they are ready, they may or may not be
>>>> >> >> >> committed individually, and even then it's very unlikely that we'll
>>>> >> >> >> want to use the supplied commit message (at minimum we'll want to
>>>> >> >> >> add
>>>> >> >> >> a "Fixes #xxxx" so Redmine updates the ticket and links the commit
>>>> >> >> >> to
>>>> >> >> >> it).
>>>> >> >> >>
>>>> >> >> >> Please follow the project's normal process and submit patches that
>>>> >> >> >> can
>>>> >> >> >> be applied with git apply.
>>>> >> >> >>
>>>> >> >> >> Thanks.
>>>> >> >> >>
>>>> >> >> >>
>>>> >> >> >> On Tue, May 23, 2017 at 5:46 PM, Shruti B Iyer <siyer@pivotal.io>
>>>> >> >> >> wrote:
>>>> >> >> >> > Hi Dave,
>>>> >> >> >> >
>>>> >> >> >> > git am is also helpful to apply a patch that was created using git
>>>> >> >> >> > format-patch, which is how we created these four patches. git am
>>>> >> >> >> > will
>>>> >> >> >> > apply
>>>> >> >> >> > the diff in the patch and also make a commit using the commit
>>>> >> >> >> > message
>>>> >> >> >> > stored
>>>> >> >> >> > in the patch. Could you try it again using git am? If it still
>>>> >> >> >> > doesn't
>>>> >> >> >> > work
>>>> >> >> >> > for you, we can try creating the diff files.
>>>> >> >> >> >
>>>> >> >> >> > Thanks,
>>>> >> >> >> > Shruti & Matt
>>>> >> >> >> >
>>>> >> >> >> > On Tue, May 23, 2017 at 5:28 PM, Dave Page <dpage@pgadmin.org>
>>>> >> >> >> > wrote:
>>>> >> >> >> >>
>>>> >> >> >> >> Hi
>>>> >> >> >> >>
>>>> >> >> >> >> git am is for applying patches from mailbox files:
>>>> >> >> >> >>
>>>> >> >> >> >> Splits mail messages in a mailbox into commit log message,
>>>> >> >> >> >> authorship
>>>> >> >> >> >> information and patches, and applies them to the current branch.
>>>> >> >> >> >>
>>>> >> >> >> >> That doesn't seem like it'll help me as a gmail user. Can you fix
>>>> >> >> >> >> the
>>>> >> >> >> >> patches to apply with git apply please?
>>>> >> >> >> >>
>>>> >> >> >> >> On Tuesday, May 23, 2017, Shruti B Iyer <siyer@pivotal.io> wrote:
>>>> >> >> >> >>>
>>>> >> >> >> >>> Hi Dave,
>>>> >> >> >> >>>
>>>> >> >> >> >>> We see the same errors when doing git apply for each patch.
>>>> >> >> >> >>> However,
>>>> >> >> >> >>> if
>>>> >> >> >> >>> you do git am for each patch, it should proceed.
>>>> >> >> >> >>>
>>>> >> >> >> >>> Thanks,
>>>> >> >> >> >>> Shruti & Matt
>>>> >> >> >> >>>
>>>> >> >> >> >>> On Tue, May 23, 2017 at 4:55 PM, Dave Page <dpage@pgadmin.org>
>>>> >> >> >> >>> wrote:
>>>> >> >> >> >>>>
>>>> >> >> >> >>>> Hi!
>>>> >> >> >> >>>>
>>>> >> >> >> >>>> Looks great! I found a few issues which I think should be
>>>> >> >> >> >>>> addressed
>>>> >> >> >> >>>> before we continue too far. Note that I haven't reviewed the
>>>> >> >> >> >>>> code
>>>> >> >> >> >>>> at
>>>> >> >> >> >>>> this stage:
>>>> >> >> >> >>>>
>>>> >> >> >> >>>> - When dragging a selection, the bounding box doesn't line up
>>>> >> >> >> >>>> with
>>>> >> >> >> >>>> the
>>>> >> >> >> >>>> bottom of the grid rows. Note that I couldn't screen shot this
>>>> >> >> >> >>>> unfortunately. It's not broken as such - just looks wrong.
>>>> >> >> >> >>>>
>>>> >> >> >> >>>> - If I copy one or more rows, I'm unable to paste them in as
>>>> >> >> >> >>>> new
>>>> >> >> >> >>>> rows
>>>> >> >> >> >>>> when editing table data.
>>>> >> >> >> >>>>
>>>> >> >> >> >>>> - The 0004 patch doesn't apply:
>>>> >> >> >> >>>>
>>>> >> >> >> >>>> (pgadmin4)snake:web dpage$ git apply
>>>> >> >> >> >>>> ~/Downloads/0004-Introduces-XCellSelectionModel.patch
>>>> >> >> >> >>>>
>>>> >> >> >> >>>>
>>>> >> >> >> >>>> /Users/dpage/Downloads/0004-Introduces-XCellSelectionModel.patch:640:
>>>> >> >> >> >>>> trailing whitespace.
>>>> >> >> >> >>>>     function scrollColumnIntoView(columnIndex) {
>>>> >> >> >> >>>>
>>>> >> >> >> >>>>
>>>> >> >> >> >>>> /Users/dpage/Downloads/0004-Introduces-XCellSelectionModel.patch:641:
>>>> >> >> >> >>>> trailing whitespace.
>>>> >> >> >> >>>>       var colspan = getColspan(row, columnIndex);
>>>> >> >> >> >>>>
>>>> >> >> >> >>>>
>>>> >> >> >> >>>> /Users/dpage/Downloads/0004-Introduces-XCellSelectionModel.patch:642:
>>>> >> >> >> >>>> trailing whitespace.
>>>> >> >> >> >>>>
>>>> >> >> >> >>>>
>>>> >> >> >> >>>>
>>>> >> >> >> >>>> /Users/dpage/Downloads/0004-Introduces-XCellSelectionModel.patch:643:
>>>> >> >> >> >>>> trailing whitespace.
>>>> >> >> >> >>>>       var left = columnPosLeft[columnIndex],
>>>> >> >> >> >>>>
>>>> >> >> >> >>>>
>>>> >> >> >> >>>> /Users/dpage/Downloads/0004-Introduces-XCellSelectionModel.patch:644:
>>>> >> >> >> >>>> trailing whitespace.
>>>> >> >> >> >>>>         right = columnPosRight[columnIndex + (colspan > 1 ?
>>>> >> >> >> >>>> colspan -
>>>> >> >> >> >>>> 1
>>>> >> >> >> >>>> : 0)],
>>>> >> >> >> >>>> error: patch failed:
>>>> >> >> >> >>>> web/pgadmin/static/vendor/slickgrid/slick.grid.js:2794
>>>> >> >> >> >>>> error: web/pgadmin/static/vendor/slickgrid/slick.grid.js: patch
>>>> >> >> >> >>>> does
>>>> >> >> >> >>>> not
>>>> >> >> >> >>>> apply
>>>> >> >> >> >>>>
>>>> >> >> >> >>>> Thanks, Dave.
>>>> >> >> >> >>>>
>>>> >> >> >> >>>>
>>>> >> >> >> >>>> On Tue, May 23, 2017 at 12:11 PM, Matthew Kleiman
>>>> >> >> >> >>>> <mkleiman@pivotal.io>
>>>> >> >> >> >>>> wrote:
>>>> >> >> >> >>>> > Hi Hackers!
>>>> >> >> >> >>>> >
>>>> >> >> >> >>>> > Attached are the updates to the query results grid, broken up
>>>> >> >> >> >>>> > into
>>>> >> >> >> >>>> > four
>>>> >> >> >> >>>> > patches.
>>>> >> >> >> >>>> >
>>>> >> >> >> >>>> >
>>>> >> >> >> >>>> > Description of the Completed Functionality After Applying All
>>>> >> >> >> >>>> > Four
>>>> >> >> >> >>>> > Patches
>>>> >> >> >> >>>> > Currently the designed behavior is somewhere between excel
>>>> >> >> >> >>>> > like
>>>> >> >> >> >>>> > behavior and
>>>> >> >> >> >>>> > not. As such we can describe the behavior as follows:
>>>> >> >> >> >>>> >
>>>> >> >> >> >>>> > Select columns by clicking on the header
>>>> >> >> >> >>>> > Select rows by clicking on the row header (column 0)
>>>> >> >> >> >>>> > You can drag and select with the mouse
>>>> >> >> >> >>>> > You can select all with ctrl+a or by clicking the upper left
>>>> >> >> >> >>>> > cell
>>>> >> >> >> >>>> > You can copy with ctrl+c or with the copy icon
>>>> >> >> >> >>>> > you can increase or decrease the size of the selected area
>>>> >> >> >> >>>> > with
>>>> >> >> >> >>>> > shift+arrow
>>>> >> >> >> >>>> > shift+arrow understands directionality, e.g. drag select from
>>>> >> >> >> >>>> > left
>>>> >> >> >> >>>> > to
>>>> >> >> >> >>>> > right
>>>> >> >> >> >>>> > differs from drag select from right to left
>>>> >> >> >> >>>> > Clicking anywhere outside of the selected area deselects the
>>>> >> >> >> >>>> > area
>>>> >> >> >> >>>> > and
>>>> >> >> >> >>>> > reselects the new cell(s) clicked on
>>>> >> >> >> >>>> >
>>>> >> >> >> >>>> > Current potentially awkward but intentional functionality
>>>> >> >> >> >>>> >
>>>> >> >> >> >>>> > When you select multiple columns/rows by clicking on the
>>>> >> >> >> >>>> > header,
>>>> >> >> >> >>>> > then
>>>> >> >> >> >>>> > press
>>>> >> >> >> >>>> > shift+arrow all but the last selected columns/rows are
>>>> >> >> >> >>>> > deselected
>>>> >> >> >> >>>> >
>>>> >> >> >> >>>> > Includes fixes for:
>>>> >> >> >> >>>> > RM Bug #2348 - On resize of first/any column in "Query
>>>> >> >> >> >>>> > Tool/View
>>>> >> >> >> >>>> > Data"
>>>> >> >> >> >>>> > will
>>>> >> >> >> >>>> > select/deselect all the rows/columns.
>>>> >> >> >> >>>> > RM Bug #2344 - ctrl+v and ctrl+c need to work
>>>> >> >> >> >>>> >
>>>> >> >> >> >>>> >
>>>> >> >> >> >>>> > Detailed Description of Each Patch
>>>> >> >> >> >>>> >
>>>> >> >> >> >>>> >
>>>> >> >> >> >>>> > 0001-Improves-user-s-ability-to-select-cells-in-query-res.patch
>>>> >> >> >> >>>> > -
>>>> >> >> >> >>>> >
>>>> >> >> >> >>>> >       - user can select columns
>>>> >> >> >> >>>> >
>>>> >> >> >> >>>> >       - user can modify column or row selection with
>>>> >> >> >> >>>> > shift+arrow
>>>> >> >> >> >>>> >
>>>> >> >> >> >>>> >       - user can select entire grid with ctrl+A or cmd+A
>>>> >> >> >> >>>> >
>>>> >> >> >> >>>> >       - user can copy from grid using keyboard shortcuts
>>>> >> >> >> >>>> >
>>>> >> >> >> >>>> > 0002-Drag-and-select-from-data-grid.patch -
>>>> >> >> >> >>>> >
>>>> >> >> >> >>>> > 0003-Removes-checkboxes-from-the-grid.patch -
>>>> >> >> >> >>>> >
>>>> >> >> >> >>>> >     -  Changes header color to grey
>>>> >> >> >> >>>> >
>>>> >> >> >> >>>> > 0004-Introduces-XCellSelectionModel.patch -
>>>> >> >> >> >>>> >
>>>> >> >> >> >>>> >     - header styles depend on the selection
>>>> >> >> >> >>>> >
>>>> >> >> >> >>>> >     - show the correct row/column when scrolling up or left
>>>> >> >> >> >>>> >
>>>> >> >> >> >>>> >     - fixes drag and drop when drop is done outside the grid
>>>> >> >> >> >>>> >
>>>> >> >> >> >>>> >
>>>> >> >> >> >>>> > Thanks,
>>>> >> >> >> >>>> >
>>>> >> >> >> >>>> > Matt & Shruti
>>>> >> >> >> >>>> >
>>>> >> >> >> >>>> >
>>>> >> >> >> >>>> >
>>>> >> >> >> >>>> >
>>>> >> >> >> >>>> >
>>>> >> >> >> >>>> > --
>>>> >> >> >> >>>> > Sent via pgadmin-hackers mailing list
>>>> >> >> >> >>>> > (pgadmin-hackers@postgresql.org)
>>>> >> >> >> >>>> > To make changes to your subscription:
>>>> >> >> >> >>>> > http://www.postgresql.org/mailpref/pgadmin-hackers
>>>> >> >> >> >>>> >
>>>> >> >> >> >>>>
>>>> >> >> >> >>>>
>>>> >> >> >> >>>>
>>>> >> >> >> >>>> --
>>>> >> >> >> >>>> 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
>>>> >> >> >
>>>> >> >> >
>>>> >> >>
>>>> >> >>
>>>> >> >>
>>>> >> >> --
>>>> >> >> Dave Page
>>>> >> >> Blog: http://pgsnake.blogspot.com
>>>> >> >> Twitter: @pgsnake
>>>> >> >>
>>>> >> >> EnterpriseDB UK: http://www.enterprisedb.com
>>>> >> >> The Enterprise PostgreSQL Company
>>>> >> >>
>>>> >> >>
>>>> >> >> --
>>>> >> >> Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
>>>> >> >> To make changes to your subscription:
>>>> >> >> http://www.postgresql.org/mailpref/pgadmin-hackers
>>>> >> >
>>>> >> >
>>>> >>
>>>> >>
>>>> >>
>>>> >> --
>>>> >> Dave Page
>>>> >> Blog: http://pgsnake.blogspot.com
>>>> >> Twitter: @pgsnake
>>>> >>
>>>> >> EnterpriseDB UK: http://www.enterprisedb.com
>>>> >> The Enterprise PostgreSQL Company
>>>> >>
>>>> >>
>>>> >> --
>>>> >> Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
>>>> >> To make changes to your subscription:
>>>> >> http://www.postgresql.org/mailpref/pgadmin-hackers
>>>> >
>>>> >
>>>>
>>>>
>>>>
>>>> --
>>>> Dave Page
>>>> Blog: http://pgsnake.blogspot.com
>>>> Twitter: @pgsnake
>>>>
>>>> EnterpriseDB UK: http://www.enterprisedb.com
>>>> The Enterprise PostgreSQL Company
>>>
>>>
>>
>



В списке pgadmin-hackers по дате отправления:

Предыдущее
От: Matthew Kleiman
Дата:
Сообщение: Re: [pgadmin-hackers] Process for Creating Translations
Следующее
От: Dave Page
Дата:
Сообщение: Re: [pgadmin-hackers] Process for Creating Translations