Fix running multiple queries & sorting when multiple tables are present in psql#41
Fix running multiple queries & sorting when multiple tables are present in psql#41misterbuckley wants to merge 13 commits intojoereynolds:masterfrom
Conversation
also psql apparently really doesn't like `\n`'s or anything with backslashes in its input, so get rid of them. probably because it uses backslashes to begin psql commands
|
Hey @misterbuckley I've written some tests already for sorting here https://github.com/joereynolds/SQHell.vim/blob/master/test/sqhell.vader#L73 Is there any chance you'd like to add some new ones for this? i.e. Create a new test where the provider is |
|
Will do |
if the cursor was on the last character before a |, the sort function would include that | in the count and thus sort the next column, rather than the one the cursor was actually in
|
Hey, there was a test failure (details below) Nice work btw! |
Huh, no wonder my test was passing locally but some of the mysql ones were failing for me (since I set g:sqh_provider = 'psql' in my vimrc). I shoulda guessed lol. How would you feel about breaking the mysql- and psql-specific tests out into their own test files, adding Before blocks that set g:sqh_provider in each, and |
misterbuckley
force-pushed
the
bug/psql-multiple-queries
branch
from
b0de4af to
40b5678
Compare
|
@misterbuckley sounds like a good idea, go for it :) |
misterbuckley
force-pushed
the
bug/psql-multiple-queries
branch
2 times, most recently
from
d4f1197 to
7b8fb30
Compare
misterbuckley
force-pushed
the
bug/psql-multiple-queries
branch
from
7b8fb30 to
beba4ca
Compare
|
OK I'm baffled. These tests all pass for me locally. Any ideas as to why they might not be passing on travis? |
|
My only guess could be the default vim version on travis, I'll pull down your branch and see if they work for me too |
|
Okay so the mysql test fails because the sort is not numeric based, it's based on a string, so you should actually be expecting Not what you currently have which is descending by number :) Also, sorting results now chops off the header and then reattaches it. My guess is because of the psql change that you also reflected in the mysql file. Here's a gif
My guess is there's a bug with how you're sorting the tables. I'm not sure why it works locally for you because I get the same output as the test (for mysql at least) |
|
That's the thing though, Edit: I think the header being moved there is because you have 2 blank lines at the end and the mysql sort function currently just takes line 4 until the (last - 1) line, so it includes the bottom line and one of the blank lines in the sort |
|
You're right about the sort formatting, it's not an issue but the sorting is definitely string based.
You could add the |
|
hey @misterbuckley I've updated vim on the build to be Vim 8 instead of god knows what it originally comes with. I've also resolved a few conflicts and merged master in. Hopefully testing will be a bit more reliable for your changes now. |
|
Alright. We're getting somewhere :D You can confirm this by clicking in here https://travis-ci.org/joereynolds/SQHell.vim And looking at the build results of each os. Linux passes with the current tests whereas mac fails on the sort. I'm trying to think of the best course of action here, I'm open to suggestions. Rather than create 2 separate test directories (1 for mac, 1 for linux) I think we should just change the sort command based on the current os being used. So in the SortResults (whatever it's called) function, we should make sure that they produce the same output on both os's. I'd probably prefer the mac sorting behaviour to be honest, but it's your call :) |
|
Hey @misterbuckley, I've fixed the tests on mac and linux on master (made -n default). If you'd like to pull down master and re-jig your tests slightly it should all work. I removed the test where we're ordering by the 'prefix' column in the table as it's pretty pointless with I'll do some more work to get it not have -n hardcoded in but for now at least you can get your postgres stuff in :D |
|
Thanks for taking care of that! 🙇 I thought I'd get a chance to check it out over the weekend and never did. Here's hoping we can finally get this in. |
|
More test failures, you can remove the test that orders on the prefix column by the way. It's pointless now that we have |
|
Hey @misterbuckley I've merged in your multiple query fix and resolved the conflicts on the branch, now all that's left is the sort stuff so hopefully that's a little bit less to do |
this will be broken until something can be done about the sort function sorting numerically by default
2 participants
In this PR:
Things to note: