From mboxrd@z Thu Jan 1 00:00:00 1970 From: Xi Shen Subject: Re: [PATCH] ob-sql.el: Support sqlcmd and cygwin environment Date: Mon, 13 Jun 2016 05:36:33 +0000 Message-ID: References: <87r3c4em1r.fsf@saiph.selenimh> <87a8isdso8.fsf@saiph.selenimh> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary=94eb2c094118018f7e0535224694 Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:57555) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bCKYK-00019b-Sv for Emacs-orgmode@gnu.org; Mon, 13 Jun 2016 01:36:50 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bCKYI-0003lq-7w for Emacs-orgmode@gnu.org; Mon, 13 Jun 2016 01:36:47 -0400 Received: from mail-oi0-x236.google.com ([2607:f8b0:4003:c06::236]:36852) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bCKYH-0003lh-W0 for Emacs-orgmode@gnu.org; Mon, 13 Jun 2016 01:36:46 -0400 Received: by mail-oi0-x236.google.com with SMTP id p204so192371317oih.3 for ; Sun, 12 Jun 2016 22:36:43 -0700 (PDT) In-Reply-To: List-Id: "General discussions about Org-mode." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org Sender: "Emacs-orgmode" To: "Emacs-orgmode@gnu.org" --94eb2c094118018f7e0535224694 Content-Type: multipart/alternative; boundary=94eb2c094118018f780535224692 --94eb2c094118018f780535224692 Content-Type: text/plain; charset=UTF-8 Hi Nicolas, Please see my updated patch. On Sun, Jun 12, 2016 at 6:37 PM Xi Shen wrote: > Hi Nicolas, > > So I will: > > - add "org-babel-sql-convert-filename", so another name...I am thinking > - remove `msosql` support. I am been playing with the options for a while, > and I could not find a way the make osql output the same format as sqlcmd > > > Thanks, > David > > > On Sun, Jun 12, 2016 at 10:12 AM Xi Shen wrote: > >> Yes, I think it is better to let upstream function to resolve the path >> for org-mode. >> >> But I have never contacted Emacs developers before. Should I go through >> the bug-gnu-emacs@gnu.org mail list? Or there's a more effective channel? >> >> >> On Sat, Jun 11, 2016 at 4:41 PM Nicolas Goaziou >> wrote: >> >>> Hello, >>> >>> Xi Shen writes: >>> >>> > According to >>> > >>> https://www.gnu.org/software/emacs/manual/html_node/elisp/Standard-File-Names.html >>> , >>> > the `convert-standard-filename` works for *nix and MS-DOS, but not >>> Cygwin >>> > environment. And I tested, it does not work. For the prefix, please >>> advice >>> > me a better one. Maybe we should path this function first? How can I >>> > patch/update a Emacs native function? >>> >>> Since there is no module in Emacs, you need to prefix functions and >>> variables according to the package, or, even better, the library they >>> belong to. >>> >>> Hence, functions and variables in "ob-sql.el" are prefixed with >>> "org-babel-sql-". >>> >>> Do you mind discussing it upstream on emacs-devel ML first? I don't >>> think this kind of function belongs to Org. If upstream has no >>> equivalent and doesn't want to add one, we might consider adding it to >>> the library. >>> >>> WDYT? >>> >>> >> > The `osql` command line tool was last updated in 2004, >>> >> > https://technet.microsoft.com/en-us/library/aa214012(v=sql.80).aspx >>> , >>> >> > and could not output the query result in a way that morden >>> >> > `org-table.el` expects. The `sqlcmd` is the preferred command line >>> >> > tool to connect the Microsoft SQL Server and it also has a Linux >>> >> > version, >>> >> > https://msdn.microsoft.com/en-us/library/hh568447(v=sql.110).aspx. >>> >> >>> >> Would it make sense to remove the msosql support then? >>> >> >>> > Yes, but I am also thinking about backward compatibility. Do you want >>> > me to create a patch to remove `msosql` support? >>> >>> AFAIU, according to your comment, "osql" output is barely usable. If you >>> think it is still usable and even used by some users, then I do not mind >>> keeping it. I just wanted to be sure we're not keeping something that is >>> not reasonable to keep. >>> >>> >> #'identity >>> >> >>> >> >>> >>> OK, but what's the difference? Care to give me a short lesson? >>> >>>Thanks! >>> >>> Not much difference, hence the "nitpick" tag. >>> >>> 'identity is a generic symbol, #'identity clearly indicates we (the >>> user, the compiler) are interested in the symbol function cell. >>> >>> In this case, it is obvious, but it is not always the case in other >>> parts of the code base, and more consistency in the right direction >>> doesn't hurt. >>> >>> >>> Regards, >>> >>> -- >>> Nicolas Goaziou >>> >>> >> >> Thanks, >> David >> >> -- >> >> Thanks, >> David S. >> > -- > > Thanks, > David S. > -- Thanks, David S. --94eb2c094118018f780535224692 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
Hi Nicolas,

Please see my updated patch= .


= On Sun, Jun 12, 2016 at 6:37 PM Xi Shen <davidshen84@gmail.com> wrote:
Hi Nicolas,

So I will:

- add "org-babel-sql-convert-filename", so= another name...I am thinking
- remove `msosql` support. I am bee= n playing with the options for a while, and I could not find a way the make= osql output the same format as sqlcmd


<= div>Thanks,
David


On Sun, Jun 12, 2016 at 10:12 AM Xi Shen <davidshen84@gmail.c= om> wrote:
= Yes, I think it is better to let upstream function to resolve the path for = org-mode.

But I have never contacted Emacs developers be= fore. Should I go through the=C2=A0bug-gnu-emacs@gnu.org= =C2=A0mail list? Or there's a more effective channel?


On S= at, Jun 11, 2016 at 4:41 PM Nicolas Goaziou <mail@nicolasgoaziou.fr> wrote:
<= /div>
Hello,

Xi Shen <davi= dshen84@gmail.com> writes:

> According to
> https://www.g= nu.org/software/emacs/manual/html_node/elisp/Standard-File-Names.html,<= br> > the `convert-standard-filename` works for *nix and MS-DOS, but not Cyg= win
> environment. And I tested, it does not work. For the prefix, please ad= vice
> me a better one. Maybe we should path this function first? How can I > patch/update a Emacs native function?

Since there is no module in Emacs, you need to prefix functions and
variables according to the package, or, even better, the library they
belong to.

Hence, functions and variables in "ob-sql.el" are prefixed with "org-babel-sql-".

Do you mind discussing it upstream on emacs-devel ML first? I don't
think this kind of function belongs to Org. If upstream has no
equivalent and doesn't want to add one, we might consider adding it to<= br> the library.

WDYT?

>> > The `osql` command line tool was last updated in 2004,
>> > https://technet.m= icrosoft.com/en-us/library/aa214012(v=3Dsql.80).aspx,
>> > and could not output the query result in a way that morden >> > `org-table.el` expects.=C2=A0 The `sqlcmd` is the preferred c= ommand line
>> > tool to connect the Microsoft SQL Server and it also has a Li= nux
>> > version,
>> > https://msdn.micros= oft.com/en-us/library/hh568447(v=3Dsql.110).aspx.
>>
>> Would it make sense to remove the msosql support then?
>>
> Yes, but I am also thinking about backward compatibility. Do you want<= br> > me to create a patch to remove `msosql` support?

AFAIU, according to your comment, "osql" output is barely usable.= If you
think it is still usable and even used by some users, then I do not mind keeping it. I just wanted to be sure we're not keeping something that i= s
not reasonable to keep.

>> #'identity
>>
>>
>>> OK, but what's the difference? Care to give me a short les= son?
>>>Thanks!

Not much difference, hence the "nitpick" tag.

'identity is a generic symbol, #'identity clearly indicates we (the=
user, the compiler) are interested in the symbol function cell.

In this case, it is obvious, but it is not always the case in other
parts of the code base, and more consistency in the right direction
doesn't hurt.


Regards,

--
Nicolas Goaziou



Thanks,
David
=C2=A0
--

Thanks,
David S.

--

Thanks,
David S.

--

Thank= s,
David S.

--94eb2c094118018f780535224692-- --94eb2c094118018f7e0535224694 Content-Type: application/octet-stream; name="0001-ob-sql.el-Support-sqlcmd-in-Cygwin-environment.patch" Content-Disposition: attachment; filename="0001-ob-sql.el-Support-sqlcmd-in-Cygwin-environment.patch" Content-Transfer-Encoding: base64 Content-ID: <1554841e2f3c42fe9881> X-Attachment-Id: 1554841e2f3c42fe9881 RnJvbSBlNjMxODU0YTBlYjkwYjBiMGUzNjFkY2I5ZTFmMWVmZTAzNDk4NGYwIE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBYaSBTaGVuIDxkYXZpZHNoZW44NEBnbWFpbC5jb20+CkRhdGU6 IFdlZCwgOCBKdW4gMjAxNiAxMzo0OTo1NCArMDgwMApTdWJqZWN0OiBbUEFUQ0hdIG9iLXNxbC5l bDogU3VwcG9ydCBzcWxjbWQgaW4gQ3lnd2luIGVudmlyb25tZW50CgoqIGxpc3Avb2Itc3FsLmVs IChvcmctYmFiZWwtc3FsLWRic3RyaW5nLW1zc3FsKTogRm9ybWF0IE1pY3Jvc29mdAogIGBzcWxj bWQnIGNvbW1hbmQgbGluZSBhcmdzLgoob3JnLWJhYmVsLXNxbC1jb252ZXJ0LXN0YW5kYXJkLWZp bGVuYW1lKTogQ29udmVydCBhIFBvc2l4IHBhdGggdG8KV2luZG93cyBsb25nIHBhdGggaW4gQ3ln d2luIGVudmlyb25tZW50LCBvciBkbyBub3RoaW5nLgoob3JnLWJhYmVsLWV4ZWN1dGU6c3FsKTog QWRkIGBtc3NxbCcgY29tbWFuZCBzdXBwb3J0IGFuZCByZW1vdmUgc3VwcG9ydApmb3IgYG1zb3Nx bCcuCgpUaGUgYG9zcWwnIGNvbW1hbmQgbGluZSB0b29sIHdhcyBsYXN0IHVwZGF0ZWQgaW4gMjAw NCwKaHR0cHM6Ly90ZWNobmV0Lm1pY3Jvc29mdC5jb20vZW4tdXMvbGlicmFyeS9hYTIxNDAxMih2 PXNxbC44MCkuYXNweCwgYW5kCmNvdWxkIG5vdCBvdXRwdXQgdGhlIHF1ZXJ5IHJlc3VsdCBpbiBh IHdheSB0aGF0IG1vcmRlbiBgb3JnLXRhYmxlLmVsJwpleHBlY3RzLiAgVGhlIGBzcWxjbWQnIGlz IHRoZSBwcmVmZXJyZWQgY29tbWFuZCBsaW5lIHRvb2wgdG8gY29ubmVjdCB0aGUKTWljcm9zb2Z0 IFNRTCBTZXJ2ZXIgYW5kIGl0IGFsc28gaGFzIGEgTGludXggdmVyc2lvbiwKaHR0cHM6Ly9tc2Ru Lm1pY3Jvc29mdC5jb20vZW4tdXMvbGlicmFyeS9oaDU2ODQ0Nyh2PXNxbC4xMTApLmFzcHguCgpU SU5ZQ0hBTkdFCi0tLQogbGlzcC9vYi1zcWwuZWwgfCAzNSArKysrKysrKysrKysrKysrKysrKysr KysrKysrKysrKystLQogMSBmaWxlIGNoYW5nZWQsIDMzIGluc2VydGlvbnMoKyksIDIgZGVsZXRp b25zKC0pCgpkaWZmIC0tZ2l0IGEvbGlzcC9vYi1zcWwuZWwgYi9saXNwL29iLXNxbC5lbAppbmRl eCA2NDg4YWZlLi5lZTA2NTBjIDEwMDY0NAotLS0gYS9saXNwL29iLXNxbC5lbAorKysgYi9saXNw L29iLXNxbC5lbApAQCAtMTAzLDYgKzEwMywyNyBAQCBQYXNzIG5pbCB0byBvbWl0IHRoYXQgYXJn LiIKICAgIk1ha2UgT3JhY2xlIGNvbW1hbmQgbGluZSBhcmdzIGZvciBkYXRhYmFzZSBjb25uZWN0 aW9uLiIKICAgKGZvcm1hdCAiJXMvJXNAJXM6JXMvJXMiIHVzZXIgcGFzc3dvcmQgaG9zdCBwb3J0 IGRhdGFiYXNlKSkKIAorKGRlZnVuIG9yZy1iYWJlbC1zcWwtZGJzdHJpbmctbXNzcWwgKGhvc3Qg dXNlciBwYXNzd29yZCBkYXRhYmFzZSkKKyAgIk1ha2Ugc3FsY21kIGNvbW1tYW5kIGxpbmUgYXJn cyBmb3IgZGF0YWJhc2UgY29ubmVjdGlvbi4KK2BzcWxjbWQnIGlzIHRoZSBwcmVmZXJyZWQgY29t bWFuZCBsaW5lIHRvb2wgdG8gYWNjZXNzIE1pY3Jvc29mdAorU1FMIFNlcnZlciBvbiBXaW5kb3dz IGFuZCBMaW51eCBwbGF0Zm9ybS4iCisgIChtYXBjb25jYXQgIydpZGVudGl0eQorCSAgICAgKGRl bHEgbmlsCisJCSAgIChsaXN0ICh3aGVuIGhvc3QgKGZvcm1hdCAiLVMgXCIlc1wiIiBob3N0KSkK KwkJCSAod2hlbiB1c2VyIChmb3JtYXQgIi1VIFwiJXNcIiIgdXNlcikpCisJCQkgKHdoZW4gcGFz c3dvcmQgKGZvcm1hdCAiLVAgXCIlc1wiIiBwYXNzd29yZCkpCisJCQkgKHdoZW4gZGF0YWJhc2Ug KGZvcm1hdCAiLWQgXCIlc1wiIiBkYXRhYmFzZSkpKSkKKwkgICAgICIgIikpCisKKyhkZWZ1biBv cmctYmFiZWwtc3FsLWNvbnZlcnQtc3RhbmRhcmQtZmlsZW5hbWUgKGZpbGUpCisgICJDb252ZXJ0 IHRoZSBmaWxlIG5hbWUgdG8gT1Mgc3RhbmRhcmQuCitJbiBDeWd3aW4gZW52aXJvbm1lbnQsIGlz IHVzZXMgQ3lnd2luIHNwZWNpZmljIGZ1bmN0aW9uIHRvCitjb252ZXJ0IHRoZSBmaWxlIG5hbWUg YW5kIGRvdWJsZSBxdW90ZSBpdC4gT3RoZXJ3aXNlLCB1c2VzIEVtYWNzCitzdGFuZGFyZCBjb252 ZXJzaW9uIGZ1bmN0aW9uLiIKKyAgKGlmIChmYm91bmRwICdjeWd3aW4tY29udmVydC1maWxlLW5h bWUtdG8td2luZG93cykKKyAgICAgIChmb3JtYXQgIlwiJXNcIiIgKGN5Z3dpbi1jb252ZXJ0LWZp bGUtbmFtZS10by13aW5kb3dzIGZpbGUpKQorICAgIChjb252ZXJ0LXN0YW5kYXJkLWZpbGVuYW1l IGZpbGUpKSkKKwogKGRlZnVuIG9yZy1iYWJlbC1leGVjdXRlOnNxbCAoYm9keSBwYXJhbXMpCiAg ICJFeGVjdXRlIGEgYmxvY2sgb2YgU3FsIGNvZGUgd2l0aCBCYWJlbC4KIFRoaXMgZnVuY3Rpb24g aXMgY2FsbGVkIGJ5IGBvcmctYmFiZWwtZXhlY3V0ZS1zcmMtYmxvY2snLiIKQEAgLTEzMSw4ICsx NTIsMTggQEAgVGhpcyBmdW5jdGlvbiBpcyBjYWxsZWQgYnkgYG9yZy1iYWJlbC1leGVjdXRlLXNy Yy1ibG9jaycuIgogCQkJCSAgICAgIChvcmctYmFiZWwtcHJvY2Vzcy1maWxlLW5hbWUgb3V0LWZp bGUpKSkKICAgICAgICAgICAgICAgICAgICAgKGBtc29zcWwgKGZvcm1hdCAib3NxbCAlcyAtcyBc Ilx0XCIgLWkgJXMgLW8gJXMiCiAJCQkJICAgICAob3IgY21kbGluZSAiIikKLQkJCQkgICAgIChv cmctYmFiZWwtcHJvY2Vzcy1maWxlLW5hbWUgaW4tZmlsZSkKLQkJCQkgICAgIChvcmctYmFiZWwt cHJvY2Vzcy1maWxlLW5hbWUgb3V0LWZpbGUpKSkKKwkJCQkgICAgIChvcmctYmFiZWwtc3FsLWNv bnZlcnQtc3RhbmRhcmQtZmlsZW5hbWUKKwkJCQkgICAgICAob3JnLWJhYmVsLXByb2Nlc3MtZmls ZS1uYW1lIGluLWZpbGUpKQorCQkJCSAgICAgKG9yZy1iYWJlbC1zcWwtY29udmVydC1zdGFuZGFy ZC1maWxlbmFtZQorCQkJCSAgICAgIChvcmctYmFiZWwtcHJvY2Vzcy1maWxlLW5hbWUgb3V0LWZp bGUpKSkpCisJCSAgICAoYG1zc3FsIChmb3JtYXQgInNxbGNtZCAlcyAtcyBcIlx0XCIgJXMgLWkg JXMgLW8gJXMiCisJCQkJICAgICAob3IgY21kbGluZSAiIikKKwkJCQkgICAgIChvcmctYmFiZWwt c3FsLWRic3RyaW5nLW1zc3FsCisJCQkJICAgICAgZGJob3N0IGRidXNlciBkYnBhc3N3b3JkIGRh dGFiYXNlKQorCQkJCSAgICAgKG9yZy1iYWJlbC1zcWwtY29udmVydC1zdGFuZGFyZC1maWxlbmFt ZQorCQkJCSAgICAgIChvcmctYmFiZWwtcHJvY2Vzcy1maWxlLW5hbWUgaW4tZmlsZSkpCisJCQkJ ICAgICAob3JnLWJhYmVsLXNxbC1jb252ZXJ0LXN0YW5kYXJkLWZpbGVuYW1lCisJCQkJICAgICAg KG9yZy1iYWJlbC1wcm9jZXNzLWZpbGUtbmFtZSBvdXQtZmlsZSkpKSkKICAgICAgICAgICAgICAg ICAgICAgKGBteXNxbCAoZm9ybWF0ICJteXNxbCAlcyAlcyAlcyA8ICVzID4gJXMiCiAJCQkJICAg IChvcmctYmFiZWwtc3FsLWRic3RyaW5nLW15c3FsCiAJCQkJICAgICBkYmhvc3QgZGJwb3J0IGRi dXNlciBkYnBhc3N3b3JkIGRhdGFiYXNlKQotLSAKMi44LjMKCg== --94eb2c094118018f7e0535224694--