Re: [O] [PATCH] ob-sql.el: Support sqlcmd and cygwin environment

[Xi Shen]

Hello Nicolas,

Please see my replies inline.

On Sat, Jun 11, 2016 at 6:06 AM Nicolas Goaziou <address@hidden> wrote:

Hello,

Xi Shen <address@hidden> 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.