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: Sat, 11 Jun 2016 02:17:52 +0000 Message-ID: References: <87r3c4em1r.fsf@saiph.selenimh> Mime-Version: 1.0 Content-Type: multipart/alternative; boundary=001a113d4800c53b4e0534f74333 Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:40226) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bBYUw-0000dd-1n for Emacs-orgmode@gnu.org; Fri, 10 Jun 2016 22:18:07 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bBYUt-0004jl-8x for Emacs-orgmode@gnu.org; Fri, 10 Jun 2016 22:18:04 -0400 Received: from mail-oi0-x235.google.com ([2607:f8b0:4003:c06::235]:35856) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bBYUt-0004jh-2I for Emacs-orgmode@gnu.org; Fri, 10 Jun 2016 22:18:03 -0400 Received: by mail-oi0-x235.google.com with SMTP id p204so137962578oih.3 for ; Fri, 10 Jun 2016 19:18:02 -0700 (PDT) In-Reply-To: <87r3c4em1r.fsf@saiph.selenimh> 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: Xi Shen , "Emacs-orgmode@gnu.org" --001a113d4800c53b4e0534f74333 Content-Type: text/plain; charset=UTF-8 Hello Nicolas, Please see my replies inline. On Sat, Jun 11, 2016 at 6:06 AM Nicolas Goaziou wrote: > Hello, > > Xi Shen writes: > > > I would like to apply this path to add sqlcmd support, and allow org-mode > > to execute and capture sqlcmd output in cygwin environment. > > Thank you for the patch. > > > I added a "platform-convert-file-name" function to convert a *nix path to > > Windows path. Should I put this function in ob-sql.el, or somewhere > > else? > > I'm surprised it doesn't exist. Wouldn't `convert-standard-filename' do > the job? > > In any case, it doesn't have a proper prefix. > > >> 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? > > Subject: [PATCH] ob-sql.el: Support sqlcmd and cygwin environment > > > > * lisp/ob-sql.el (org-babel-sql-dbstring-mssql): Format Microsoft > > sqlcmd command line args. > > > > * lisp/ob-sql.el (platform-convert-file-name): Convert a *nix path to > > Windows long path in Cygwin environment, or do nothing. > > > > * lisp/ob-sql.el (org-babel-execute:sql): Add `mssql` command support. > > For both `msosql` and `mssql` the `input` and `output` path will be > > mapped by `platform-convert-file-name` > > You can write it like > > * lisp/ob-sql.el (org-babel-sql-dbstring-mssql): Format Microsoft > sqlcmd command line args. > (platform-convert-file-name): Convert a *nix path to Windows long path > in Cygwin environment, or do nothing. > (org-babel-execute:sql): Add `mssql' command support. For both `msosql' > and `mssql` the `input` and `output' path will be mapped by > `platform-convert-file-name' > > >> Sure, I will update the ChangeLog entry. > > 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? > > +(defun org-babel-sql-dbstring-mssql (host user password database) > > + "Make Microsoft sqlcmd commmand line args for database > > +connection." > > The first sentence has to fit on a single line. > > > Will do, thanks~ > > + (mapconcat 'identity > > Nit-pick: > > #'identity > > >> OK, but what's the difference? Care to give me a short lesson? Thanks! > > + "In Cygwin environment convert the file path into Windows long > > +path and quote it." > > + (if (fboundp 'cygwin-convert-file-name-to-windows) > > + (format "\"%s\"" (cygwin-convert-file-name-to-windows file)) > > + file)) > > See above. > > Regards, > > -- > Nicolas Goaziou > Thanks, David -- Thanks, David S. --001a113d4800c53b4e0534f74333 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
Hello Nicolas,

Please see my replies in= line.

On Sat, Jun 11, 20= 16 at 6:06 AM Nicolas Goaziou <mail@nicolasgoaziou.fr> wrote:
Hello,

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

> I would like to apply this path to add sqlcmd support, and allow org-m= ode
> to execute and capture sqlcmd output in cygwin environment.

Thank you for the patch.

> I added a "platform-convert-file-name" function to convert a= *nix path to
> Windows path. Should I put this function in ob-sql.el, or somewhere > else?

I'm surprised it doesn't exist. Wouldn't `convert-standard-file= name' do
the job?

In any case, it doesn't have a proper prefix.


>> 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?
=C2=A0
> Subject: [PATCH] ob-sql.el: Support sqlcmd and cygwin environment
>
> * lisp/ob-sql.el (org-babel-sql-dbstring-mssql): Format Microsoft
>=C2=A0 =C2=A0sqlcmd command line args.
>
> * lisp/ob-sql.el (platform-convert-file-name): Convert a *nix path to<= br> >=C2=A0 =C2=A0Windows long path in Cygwin environment, or do nothing. >
> * lisp/ob-sql.el (org-babel-execute:sql): Add `mssql` command support.=
>=C2=A0 =C2=A0For both `msosql` and `mssql` the `input` and `output` pat= h will be
>=C2=A0 =C2=A0mapped by `platform-convert-file-name`

You can write it like

=C2=A0* lisp/ob-sql.el (org-babel-sql-dbstring-mssql): Format Microsoft
=C2=A0 =C2=A0sqlcmd command line args.
=C2=A0(platform-convert-file-name): Convert a *nix path to Windows long pat= h
=C2=A0in Cygwin environment, or do nothing.
=C2=A0(org-babel-execute:sql): Add `mssql' command support. For both `m= sosql'
=C2=A0and `mssql` the `input` and `output' path will be mapped by
=C2=A0`platform-convert-file-name'

>> Sure, I will update the ChangeLog entry.
=C2=A0
> The `osql` command line tool was last updated in 2004,
> https://technet.microsoft.= 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 command li= ne
> tool to connect the Microsoft SQL Server and it also has a Linux
> version,
> https://msdn.microsoft.com/e= n-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 c= ompatibility. Do you want me to create a patch to remove `msosql` support?<= /div>
=C2=A0
> +(defun org-babel-sql-dbstring-mssql (host user password database)
> +=C2=A0 "Make Microsoft sqlcmd commmand line args for database > +connection."

The first sentence has to fit on a single line.

> Will do, thanks~
=C2=A0
> +=C2=A0 (mapconcat 'identity

Nit-pick:

#'identity


>> OK, but what's the differ= ence? Care to give me a short lesson? Thanks!
=C2=A0
> +=C2=A0 "In Cygwin environment convert the file path into Windows= long
> +path and quote it."
> +=C2=A0 (if (fboundp 'cygwin-convert-file-name-to-windows)
> +=C2=A0 =C2=A0 =C2=A0 (format "\"%s\"" (cygwin-con= vert-file-name-to-windows file))
> +=C2=A0 =C2=A0 file))

See above.

Regards,

--
Nicolas Goaziou


Thanks,<= /div>
David
=C2=A0
--=

Thanks,
David S.

--001a113d4800c53b4e0534f74333--