Skip to content

Feature/insert row key#44

Open
jbyte wants to merge 9 commits intojoereynolds:masterfrom
jbyte:feature/insert-row-key
Open

Feature/insert row key#44
jbyte wants to merge 9 commits intojoereynolds:masterfrom
jbyte:feature/insert-row-key

Conversation

@jbyte
Copy link
Contributor

@jbyte jbyte commented Dec 8, 2017

Description (what is it you've done, has anything changed?)

  • updated mysql#PostBufferFormat so it automatically deletes all lines before the expected output (everything before a line containing +----- for all SQH filetypes except SQHInsert, or everything before a line containing , for SQHInsert filetype) -- this could probably use some more tweeking
  • added function mysql#GetTableInfoFromQuery(query): returns list [database, table] names (used in mysql#EditRow and mysql#InsertRow
  • added function mysql#CreateInsertFromCSV(): creates an INSERT sql statement from the csv in the current buffer
  • added function mysql#InsertRow(): opens a SQHInsert buffer with the headings in csv format
  • added key mapping for calling provider non-specific xxxx#InsertRow function
  • added simple vader tests for mysql#CreateInsertFromCSV and mysql#CreateUpdateFromCSV

Checklist

  • I have written my code in a way that it can be easily tested
  • I have written tests for my code
  • I have documented new features in doc/sqhell.txt
  • I have added information to the README (where relevant)
  • I have ran the automated tests and performed manual tests.

@jbyte
Copy link
Contributor Author

jbyte commented Dec 9, 2017

Well I also fixed the travis issue where it didn't use the correct branch/pull request for Vader tests.
Although the mac tests still fail because the macOS sort works different than the linux one.

@joereynolds
Copy link
Owner

Hey @jbyte

All looks good, I'm going to fix the Mac Vs Linux sort bug before merging any pull requests

Once I've done that I'll comment on here

@jbyte
Copy link
Contributor Author

jbyte commented Dec 9, 2017

I'm fine with that.

@joereynolds
Copy link
Owner

Hey @jbyte,

I've fixed the tests/code, can you pull down master and try again?

Vim is already installed, and no need to clone the repo it does that by
default (it also checkouts the correct branch/pull request).
The installed one was an old 7.4 version
@jbyte jbyte force-pushed the feature/insert-row-key branch from a112182 to 0c647a5 Compare December 10, 2017 15:21
@jbyte
Copy link
Contributor Author

jbyte commented Dec 10, 2017

The tests are fine now 👍

if(&ft == 'testType')
return
endif
if(&ft == 'SQHInsert')
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these are modifying the jump list, meaning if I do <c-o> or <c-i> I need to press them a few times, can we avoid adding to the jump list?

@joereynolds
Copy link
Owner

When I try to ZZ on an i in the SQHResult buffer with this data

id,prefix,location,date,artist,url
"22", "With", "Wales", "2018-07-03", "The Three Stooges", "http://google.com"

I get

Incorrect number of values.
Expected: 6, got: 11.

The formating of the buffers added jumps to the jump list.
Fixed by adding _keepjumps_.

When saving an SQHInsert the plugin emited an error if there were
any spaces between commas.
Fixed by improving the filter call.
@jbyte
Copy link
Contributor Author

jbyte commented Dec 10, 2017

It should be fixed now.

@joereynolds
Copy link
Owner

I think now that we do everything in one buffer, your calls to :bd can be removed otherwise we get some undesirable jumps. For example if I do the following

SQHDatabase -> SQHTable -> SQHResult -> e to edit result -> <c-o>

Instead of going back to the SQHResult as expected, it goes to SQHTable

This also happens on the i to insert a new record.

Looking good so far though :D

@jbyte
Copy link
Contributor Author

jbyte commented Dec 11, 2017

Well I wouldn't remove them, but change them. Because they are used for "reloading" the buffer.
Example:
SQHResult -> e to edit -> edit data -> ZZ to save and close -> updated SQHResult
I'll just change the :bd to something like dG and put newresults

Edit:
Never mind it's fine to remove them, I'll just move the "reloading" functionality to ZZ in SQHResult

Edit2:
The :bd in mysql#EditRow and mysql#InsertRow should be changed and moved to ZZ.
The :bd in mysql#DropDatabase and mysql#DropTableFromDatabase should be changed in place.

But in the end I think this would need quite a bit of refactoring so maybe should be its own PR?

@jbyte
Copy link
Contributor Author

jbyte commented Dec 11, 2017

Thoughts on the refactoring: #46.

@joereynolds
Copy link
Owner

Sounds good to me, we can test the query generation better this way too.

Do we need to delete all lines in the buffer each time? We use enew! inside the InsertResultsToNewBuffer function anyway so the previous buffer would be removed (though still available in the jump list I guess).

Also it sounds like the pros outweight the cons so feel free to take a stab it, I would definitely like a cleaner codebase for this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants