search

komsit37 / sublime-q

Sublime Text Plugin for q/kdb
MIT License
24 stars 9 forks source link

Update to qPython 2.0 and add support for utf-8 #28

Closed atf1206 closed 4 years ago

atf1206 commented 4 years ago

This change is focused on updating to qPython 2.0. All code changes below should just be either:

  1. Diffs from current qPython to 2.0;
  2. Add encoding = 'UTF-8' to the qconnection init; and
  3. Patch _write_string to support properly encoding strings to send to kdb. Check out the PR against the qPython repo here: https://github.com/exxeleron/qPython/pull/77

Although this is hopefully straightforward, I recommend testing it for a while before merging since it is such a core part of sublime-q.

Also, a couple things that are NOT changed (yet?): A. Did not change decoding of the strings in sublime-q, so if you send "õ" you get back "\303\265" (this is how kdb does it, but we could parse it back to "õ" if we wanted). B. I noticed a small warning, myLocale=locale.setlocale(category=locale.LC_ALL, locale="en_GB.UTF-8"); File "./python3.3/locale.py", line 573, in setlocale locale.Error: unsupported locale setting I believe that since locale is an optional arg, it should just be myLocale=locale.setlocale(category=locale.LC_ALL) but have not included that change here.

komsit37 commented 4 years ago

This is awesome. I have tested it a bit and all looks good to me so far. I think the change should be ok since most of it is just upgrading qPython.

So, let me know if you are happy with this then I can merge it.

Regarding A) yes, that's how kdb's .Q.s does it. I don't work with non-English so it's not a problem for me. But I imagine it would be helpful if you can display it correctly in sublime text. We can do this in a separate PR if needed.

B) Where do you see this warning? I can't find it in the console.

atf1206 commented 4 years ago

I think it is good as-is. Seems to be close to what you see at the REPL, including:

q)`$"ééé"
`ééé

re: B) It is not a big problem, and in fact I do not expect you to see the error since your machine likely supports en_GB.UTF-8 whereas mine does not. That's why I was suggesting to just let it default to the user's setting. But let's make it a separate PR for the future.

komsit37 commented 4 years ago

Great, I have merged it!