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 06:30:32 +0000 Message-ID: References: <87r3c4em1r.fsf@saiph.selenimh> <87a8isdso8.fsf@saiph.selenimh> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary=001a113d6e8e0d24bb05352307b4 Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:36673) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bCLOW-0005WK-Ls for Emacs-orgmode@gnu.org; Mon, 13 Jun 2016 02:30:46 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bCLOU-0004th-Sj for Emacs-orgmode@gnu.org; Mon, 13 Jun 2016 02:30:44 -0400 Received: from mail-oi0-x22f.google.com ([2607:f8b0:4003:c06::22f]:35367) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bCLOU-0004tb-Jy for Emacs-orgmode@gnu.org; Mon, 13 Jun 2016 02:30:42 -0400 Received: by mail-oi0-x22f.google.com with SMTP id w5so114611610oib.2 for ; Sun, 12 Jun 2016 23:30:42 -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" --001a113d6e8e0d24bb05352307b4 Content-Type: multipart/alternative; boundary=001a113d6e8e0d24b405352307b2 --001a113d6e8e0d24b405352307b2 Content-Type: text/plain; charset=UTF-8 Hi Nicolas, I think I uploaded the wrong patch. Sorry~ Please check this one. On Mon, Jun 13, 2016 at 1:36 PM Xi Shen wrote: > 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. > -- Thanks, David S. --001a113d6e8e0d24b405352307b2 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
Hi Nicolas,

I think I uploaded th= e wrong patch. Sorry~=C2=A0Please check thi= s one.


= On Mon, Jun 13, 2016 at 1:36 PM Xi Shen <davidshen84@gmail.com> wrote:
Hi Nicolas,

Please see my= updated patch.


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

So I will:

- add "org-babel-sql-convert-file= name", so another name...I am thinking
- remove `msosql` sup= port. 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 X= i Shen <david= shen84@gmail.com> 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=C2=A0bug-gnu-emac= s@gnu.org=C2=A0mail list? Or there's a more effective channel?


On Sat, Jun 11, 2016 at 4:41 PM Nicolas Goaziou <mail@nicolasgoaziou.fr>= ; wrote:
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.

--

Thanks,
David S.

--001a113d6e8e0d24b405352307b2-- --001a113d6e8e0d24bb05352307b4 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: <1554873b687c42fe9881> X-Attachment-Id: 1554873b687c42fe9881 RnJvbSAyYTJkMTc5MmYyMmQyOTE3YmI5ZmY2MWU3ZWI3NTYwODVmMzhkYTJiIE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBYaSBTaGVuIDxkYXZpZHNoZW44NEBnbWFpbC5jb20+CkRhdGU6 IFdlZCwgOCBKdW4gMjAxNiAxMzo0OTo1NCArMDgwMApTdWJqZWN0OiBbUEFUQ0hdIG9iLXNxbC5l bDogU3VwcG9ydCBzcWxjbWQgaW4gQ3lnd2luIGVudmlyb25tZW50CgoqIGxpc3Avb2Itc3FsLmVs IChvcmctYmFiZWwtc3FsLWRic3RyaW5nLW1zc3FsKTogRm9ybWF0IE1pY3Jvc29mdAogIGBzcWxj bWQnIGNvbW1hbmQgbGluZSBhcmdzLgoob3JnLWJhYmVsLXNxbC1jb252ZXJ0LXN0YW5kYXJkLWZp bGVuYW1lKTogQ29udmVydCBhIFBvc2l4IHBhdGggdG8KV2luZG93cyBsb25nIHBhdGggaW4gQ3ln d2luIGVudmlyb25tZW50LCBvciBkbyBub3RoaW5nLgoob3JnLWJhYmVsLWV4ZWN1dGU6c3FsKTog QWRkIGBtc3NxbCcgY29tbWFuZCBzdXBwb3J0IGFuZCByZW1vdmUgc3VwcG9ydApmb3IgYG1zb3Nx bCcuCgpUaGUgYG9zcWwnIGNvbW1hbmQgbGluZSB0b29sIHdhcyBsYXN0IHVwZGF0ZWQgaW4gMjAw NCwKaHR0cHM6Ly90ZWNobmV0Lm1pY3Jvc29mdC5jb20vZW4tdXMvbGlicmFyeS9hYTIxNDAxMih2 PXNxbC44MCkuYXNweCwgYW5kCmNvdWxkIG5vdCBvdXRwdXQgdGhlIHF1ZXJ5IHJlc3VsdCBpbiBh IHdheSB0aGF0IG1vcmRlbiBgb3JnLXRhYmxlLmVsJwpleHBlY3RzLiAgVGhlIGBzcWxjbWQnIGlz IHRoZSBwcmVmZXJyZWQgY29tbWFuZCBsaW5lIHRvb2wgdG8gY29ubmVjdCB0aGUKTWljcm9zb2Z0 IFNRTCBTZXJ2ZXIgYW5kIGl0IGFsc28gaGFzIGEgTGludXggdmVyc2lvbiwKaHR0cHM6Ly9tc2Ru Lm1pY3Jvc29mdC5jb20vZW4tdXMvbGlicmFyeS9oaDU2ODQ0Nyh2PXNxbC4xMTApLmFzcHguCgpU SU5ZQ0hBTkdFCi0tLQogbGlzcC9vYi1zcWwuZWwgfCAzMSArKysrKysrKysrKysrKysrKysrKysr KysrKysrLS0tCiAxIGZpbGUgY2hhbmdlZCwgMjggaW5zZXJ0aW9ucygrKSwgMyBkZWxldGlvbnMo LSkKCmRpZmYgLS1naXQgYS9saXNwL29iLXNxbC5lbCBiL2xpc3Avb2Itc3FsLmVsCmluZGV4IDY0 ODhhZmUuLjc0Y2QzOWYgMTAwNjQ0Ci0tLSBhL2xpc3Avb2Itc3FsLmVsCisrKyBiL2xpc3Avb2It c3FsLmVsCkBAIC0xMDMsNiArMTAzLDI3IEBAIFBhc3MgbmlsIHRvIG9taXQgdGhhdCBhcmcuIgog ICAiTWFrZSBPcmFjbGUgY29tbWFuZCBsaW5lIGFyZ3MgZm9yIGRhdGFiYXNlIGNvbm5lY3Rpb24u IgogICAoZm9ybWF0ICIlcy8lc0Alczolcy8lcyIgdXNlciBwYXNzd29yZCBob3N0IHBvcnQgZGF0 YWJhc2UpKQogCisoZGVmdW4gb3JnLWJhYmVsLXNxbC1kYnN0cmluZy1tc3NxbCAoaG9zdCB1c2Vy IHBhc3N3b3JkIGRhdGFiYXNlKQorICAiTWFrZSBzcWxjbWQgY29tbW1hbmQgbGluZSBhcmdzIGZv ciBkYXRhYmFzZSBjb25uZWN0aW9uLgorYHNxbGNtZCcgaXMgdGhlIHByZWZlcnJlZCBjb21tYW5k IGxpbmUgdG9vbCB0byBhY2Nlc3MgTWljcm9zb2Z0CitTUUwgU2VydmVyIG9uIFdpbmRvd3MgYW5k IExpbnV4IHBsYXRmb3JtLiIKKyAgKG1hcGNvbmNhdCAjJ2lkZW50aXR5CisJICAgICAoZGVscSBu aWwKKwkJICAgKGxpc3QgKHdoZW4gaG9zdCAoZm9ybWF0ICItUyBcIiVzXCIiIGhvc3QpKQorCQkJ ICh3aGVuIHVzZXIgKGZvcm1hdCAiLVUgXCIlc1wiIiB1c2VyKSkKKwkJCSAod2hlbiBwYXNzd29y ZCAoZm9ybWF0ICItUCBcIiVzXCIiIHBhc3N3b3JkKSkKKwkJCSAod2hlbiBkYXRhYmFzZSAoZm9y bWF0ICItZCBcIiVzXCIiIGRhdGFiYXNlKSkpKQorCSAgICAgIiAiKSkKKworKGRlZnVuIG9yZy1i YWJlbC1zcWwtY29udmVydC1zdGFuZGFyZC1maWxlbmFtZSAoZmlsZSkKKyAgIkNvbnZlcnQgdGhl IGZpbGUgbmFtZSB0byBPUyBzdGFuZGFyZC4KK0luIEN5Z3dpbiBlbnZpcm9ubWVudCwgaXMgdXNl cyBDeWd3aW4gc3BlY2lmaWMgZnVuY3Rpb24gdG8KK2NvbnZlcnQgdGhlIGZpbGUgbmFtZSBhbmQg ZG91YmxlIHF1b3RlIGl0LiBPdGhlcndpc2UsIHVzZXMgRW1hY3MKK3N0YW5kYXJkIGNvbnZlcnNp b24gZnVuY3Rpb24uIgorICAoaWYgKGZib3VuZHAgJ2N5Z3dpbi1jb252ZXJ0LWZpbGUtbmFtZS10 by13aW5kb3dzKQorICAgICAgKGZvcm1hdCAiXCIlc1wiIiAoY3lnd2luLWNvbnZlcnQtZmlsZS1u YW1lLXRvLXdpbmRvd3MgZmlsZSkpCisgICAgKGNvbnZlcnQtc3RhbmRhcmQtZmlsZW5hbWUgZmls ZSkpKQorCiAoZGVmdW4gb3JnLWJhYmVsLWV4ZWN1dGU6c3FsIChib2R5IHBhcmFtcykKICAgIkV4 ZWN1dGUgYSBibG9jayBvZiBTcWwgY29kZSB3aXRoIEJhYmVsLgogVGhpcyBmdW5jdGlvbiBpcyBj YWxsZWQgYnkgYG9yZy1iYWJlbC1leGVjdXRlLXNyYy1ibG9jaycuIgpAQCAtMTI5LDEwICsxNTAs MTQgQEAgVGhpcyBmdW5jdGlvbiBpcyBjYWxsZWQgYnkgYG9yZy1iYWJlbC1leGVjdXRlLXNyYy1i bG9jaycuIgogCQkJCSAgICAgIChvciBjbWRsaW5lICIiKQogCQkJCSAgICAgIChvcmctYmFiZWwt cHJvY2Vzcy1maWxlLW5hbWUgaW4tZmlsZSkKIAkJCQkgICAgICAob3JnLWJhYmVsLXByb2Nlc3Mt ZmlsZS1uYW1lIG91dC1maWxlKSkpCi0gICAgICAgICAgICAgICAgICAgIChgbXNvc3FsIChmb3Jt YXQgIm9zcWwgJXMgLXMgXCJcdFwiIC1pICVzIC1vICVzIgorCQkgICAgKGBtc3NxbCAoZm9ybWF0 ICJzcWxjbWQgJXMgLXMgXCJcdFwiICVzIC1pICVzIC1vICVzIgogCQkJCSAgICAgKG9yIGNtZGxp bmUgIiIpCi0JCQkJICAgICAob3JnLWJhYmVsLXByb2Nlc3MtZmlsZS1uYW1lIGluLWZpbGUpCi0J CQkJICAgICAob3JnLWJhYmVsLXByb2Nlc3MtZmlsZS1uYW1lIG91dC1maWxlKSkpCisJCQkJICAg ICAob3JnLWJhYmVsLXNxbC1kYnN0cmluZy1tc3NxbAorCQkJCSAgICAgIGRiaG9zdCBkYnVzZXIg ZGJwYXNzd29yZCBkYXRhYmFzZSkKKwkJCQkgICAgIChvcmctYmFiZWwtc3FsLWNvbnZlcnQtc3Rh bmRhcmQtZmlsZW5hbWUKKwkJCQkgICAgICAob3JnLWJhYmVsLXByb2Nlc3MtZmlsZS1uYW1lIGlu LWZpbGUpKQorCQkJCSAgICAgKG9yZy1iYWJlbC1zcWwtY29udmVydC1zdGFuZGFyZC1maWxlbmFt ZQorCQkJCSAgICAgIChvcmctYmFiZWwtcHJvY2Vzcy1maWxlLW5hbWUgb3V0LWZpbGUpKSkpCiAg ICAgICAgICAgICAgICAgICAgIChgbXlzcWwgKGZvcm1hdCAibXlzcWwgJXMgJXMgJXMgPCAlcyA+ ICVzIgogCQkJCSAgICAob3JnLWJhYmVsLXNxbC1kYnN0cmluZy1teXNxbAogCQkJCSAgICAgZGJo b3N0IGRicG9ydCBkYnVzZXIgZGJwYXNzd29yZCBkYXRhYmFzZSkKLS0gCjIuOC4zCgo= --001a113d6e8e0d24bb05352307b4--