Closed mtxr closed 6 years ago
What a coincidence - I was researching this topic for a few days now and made some progress too )).
With trial and error, I actually understood that sqlcmd
tries to read the input, but unlike on windows, sqlcmd
on Mac (and most probably on Linux too) does not execute the statements unless there is a trailing GO
.
Also this trailing message:
Sqlcmd: Warning: The last operation was terminated because the user pressed CTRL+C.
can be eliminated by appending QUIT
at the end queries.
@mtxr I'm still doing some testing on different platforms and I think in next few days I can propose a much cleaner way to implement it if you don't mind.
That's true!
I forgot the annoying GO issue :(
On vscode version, it's not needed though.
Both solutions are ugly, but we can make it work
After some reading, I have some points we should consider:
Using GO statement, we should parse user query to check if user is using the GO statement, something I don't really like, because it increase code complexity
Using a file, the code gets very ugly, we can have some problems running two queries simultaneously, it also increase code complexity.
To be honest, I don't know which one is better. What's your opinion?
I've been thinking about this for a while and the best solution that I've come up with is this:
Add trailing GO
statement in the end of query regardless, if user put a GO
in the end or not: I've done some testing and it works like a charm on Windows, Mac and Linux.
To do this we can create a new configurable item in settings cli_options->mssql->after
similar to before
option:
https://github.com/mtxr/SQLTools/blob/db7aff27612d4303bcc727a4e0256177cd01c3e4/SQLTools.sublime-settings#L189-L191
So we would have something like this:
"mssql": {
"options": [],
"before": [],
"after": ["GO", "QUIT"],
... ...
Benefits:
sqlcmd
works as expectedWhat do you think about this?
I think you are a genius!
But It should be activated by default on MacOS, or we should create a warning/info in the docs to easy let new users know they should add it there.
Go ahead, I think that we can close this PR now.
Nice solution!
The nice thing about this solution is that user does not have to add anything on MacOS, it would work the same on both Windows and Mac.
Here is an example:
Given the whole query text after we apply "after"
would be this:
SELECT 'hello @mtxr' as greetings; -- entered by USER
GO -- entered by USER
GO -- SQLTools added "after"
QUIT -- SQLTools added "after"
or this
SELECT 'hello @mtxr' as greetings; -- entered by USER
GO -- SQLTools added "after"
QUIT -- SQLTools added "after"
The result is the same. We still get a normal output on both platforms.
I'm looking at before
code and from what I see it is not applied in all executed queries (I guess it was added for one of RDBMS) to support specific output of specific queries.
So I guess we will have to fix it so it is always executed and review the existing usage of before
SQL.
Most probably we will have to introduce a query specific 'before' and 'after' as well to be able to reproduce the way it works now (e.g. Oracle uses before
config item, which is applied only to 'Execute', 'Show Records' and 'Explain' and not applied to internal queries 'Get List of Tables', 'Get List of Columns').
So this new pull request will have to make all those fixes and Settings adjustments.
It will take a bit longer to get this right, but on a better side, we will get a more consistent and predictable settings behavior.
I know this is not pretty as it should be, but at least it adds the support for our extension.
Attempt to fix #120.
@tkopets @mkormendy @ryanjmclaughlin could you please download this branch and test too?
I've tested on my machine, but it would be better to check if it's fully working for everyone.
Thanks@