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 <reckha...@pivotal.io> wrote: > Awesome, thanks. > > -- Rob > > On Thu, Jun 1, 2017 at 11:13 AM, Surinder Kumar < > surinder.ku...@enterprisedb.com> wrote: > >> Hi Robert, >> >> On Jun 1, 2017 8:22 PM, "Robert Eckhardt" <reckha...@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.ku...@enterprisedb.com> wrote: >> >> >> >> Hi Joao >> >> >> >> On Wed, May 31, 2017 at 1:19 AM, Joao Pedro De Almeida Pereira < >> jdealmeidapere...@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 <dp...@pgadmin.org> wrote: >> >>>> >> >>>> On Sat, May 27, 2017 at 9:02 AM, Surinder Kumar >> >>>> <surinder.ku...@enterprisedb.com> wrote: >> >>>> > Hi Dave, >> >>>> > On Sat, May 27, 2017 at 3:07 AM, Dave Page <dp...@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 >> >>>> >> <jdealmeidapere...@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 <dp...@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.cellselect >> ionmodel.js:94 >> >>>> >> >> error: >> >>>> >> >> >> >>>> >> >> web/pgadmin/static/vendor/slickgrid/plugins/slick.cellselect >> ionmodel.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.cellselect >> ionmodel.js:94 >> >>>> >> >> error: >> >>>> >> >> >> >>>> >> >> web/pgadmin/static/vendor/slickgrid/plugins/slick.cellselect >> ionmodel.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.columnpic >> ker.js >> >>>> >> >> patching file >> >>>> >> >> web/pgadmin/static/vendor/slickgrid/plugins/slick.cellcopyma >> nager.js >> >>>> >> >> patching file >> >>>> >> >> >> >>>> >> >> web/pgadmin/static/vendor/slickgrid/plugins/slick.cellextern >> alcopymanager.js >> >>>> >> >> patching file >> >>>> >> >> web/pgadmin/static/vendor/slickgrid/plugins/slick.cellranges >> elector.js >> >>>> >> >> patching file >> >>>> >> >> web/pgadmin/static/vendor/slickgrid/plugins/slick.cellselect >> ionmodel.js >> >>>> >> >> Hunk #3 succeeded at 95 with fuzz 2. >> >>>> >> >> patching file >> >>>> >> >> web/pgadmin/static/vendor/slickgrid/plugins/slick.headerbutt >> ons.js >> >>>> >> >> patching file >> >>>> >> >> web/pgadmin/static/vendor/slickgrid/plugins/slick.headermenu >> .js >> >>>> >> >> patching file >> >>>> >> >> web/pgadmin/static/vendor/slickgrid/plugins/slick.rowselecti >> onmodel.js >> >>>> >> >> patching file >> >>>> >> >> web/pgadmin/static/vendor/slickgrid/slick-default-theme.css >> >>>> >> >> patching file web/pgadmin/static/vendor/slic >> kgrid/slick.core.js >> >>>> >> >> patching file web/pgadmin/static/vendor/slic >> kgrid/slick.dataview.js >> >>>> >> >> patching file web/pgadmin/static/vendor/slic >> kgrid/slick.editors.js >> >>>> >> >> patching file web/pgadmin/static/vendor/slic >> kgrid/slick.formatters.js >> >>>> >> >> patching file web/pgadmin/static/vendor/slic >> kgrid/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/slic >> kgrid/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.groupitemmetadatap >> rovider.js >> >>>> >> >> patching file >> >>>> >> >> web/pgadmin/static/vendor/slickgrid/slick.remotemodel-yahoo.js >> >>>> >> >> patching file web/pgadmin/static/vendor/slic >> kgrid/slick.remotemodel.js >> >>>> >> >> >> >>>> >> >> On Thu, May 25, 2017 at 2:35 PM, Shruti B Iyer < >> si...@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 < >> dp...@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 < >> si...@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 < >> dp...@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 < >> si...@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 < >> dp...@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-In >> troduces-XCellSelectionModel.patch:640: >> >>>> >> >> >> >>>> trailing whitespace. >> >>>> >> >> >> >>>> function scrollColumnIntoView(columnIndex) { >> >>>> >> >> >> >>>> >> >>>> >> >> >> >>>> >> >>>> >> >> >> >>>> /Users/dpage/Downloads/0004-In >> troduces-XCellSelectionModel.patch:641: >> >>>> >> >> >> >>>> trailing whitespace. >> >>>> >> >> >> >>>> var colspan = getColspan(row, columnIndex); >> >>>> >> >> >> >>>> >> >>>> >> >> >> >>>> >> >>>> >> >> >> >>>> /Users/dpage/Downloads/0004-In >> troduces-XCellSelectionModel.patch:642: >> >>>> >> >> >> >>>> trailing whitespace. >> >>>> >> >> >> >>>> >> >>>> >> >> >> >>>> >> >>>> >> >> >> >>>> >> >>>> >> >> >> >>>> /Users/dpage/Downloads/0004-In >> troduces-XCellSelectionModel.patch:643: >> >>>> >> >> >> >>>> trailing whitespace. >> >>>> >> >> >> >>>> var left = columnPosLeft[columnIndex], >> >>>> >> >> >> >>>> >> >>>> >> >> >> >>>> >> >>>> >> >> >> >>>> /Users/dpage/Downloads/0004-In >> troduces-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 >> >>>> >> >> >> >>>> <mklei...@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-t >> o-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 >> >>> >> >>> >> >> >> > >> >> >