From: Xi Shen <davidshen84@gmail.com>
To: "Emacs-orgmode@gnu.org" <Emacs-orgmode@gnu.org>
Subject: Re: [PATCH] ob-sql.el: Support sqlcmd and cygwin environment
Date: Tue, 14 Jun 2016 13:02:38 +0000 [thread overview]
Message-ID: <CANO68EN+bZ-CV35vd7_h7bBJDY0VK2NNqBdyFwRMEr26Jecz5A@mail.gmail.com> (raw)
In-Reply-To: <87vb1c0yyn.fsf@saiph.selenimh>
[-- Attachment #1: Type: text/plain, Size: 1374 bytes --]
Hi Nicholas,
Sure, I will add the news and declaration.
The "format" is required to quote the filename, so the spaces won't
surprise sqlcmd. Maybe the arguments will be quoted when passing to the
command?
On Tue, Jun 14, 2016, 19:52 Nicolas Goaziou <mail@nicolasgoaziou.fr> wrote:
> Hello,
>
> Xi Shen <davidshen84@gmail.com> writes:
>
> > I think I uploaded the wrong patch. Sorry~ Please check this one.
>
> It looks good. Thank you. I have one minor comment about it, see below.
>
> Also, could you provide an entry for ORG-NEWS since this is a user
> visible change?
>
> > +(defun org-babel-sql-convert-standard-filename (file)
> > + "Convert the file name to OS standard.
> > +In Cygwin environment, is uses Cygwin specific function to
> > +convert the file name and double quote it. Otherwise, uses Emacs
> > +standard conversion function."
> > + (if (fboundp 'cygwin-convert-file-name-to-windows)
> > + (format "\"%s\"" (cygwin-convert-file-name-to-windows file))
> > + (convert-standard-filename file)))
>
> `cygwin-convert-file-name-to-windows' is going to generate a compilation
> warning. You need to declare it with `declare-function' at the top of
> the file.
>
> I'm also surprised that the function above doesn't return a string
> already. Are you sure the `format' part is needed?
>
> Regards,
>
> --
> Nicolas Goaziou
>
--
Thanks,
David S.
[-- Attachment #2: Type: text/html, Size: 2058 bytes --]
next prev parent reply other threads:[~2016-06-14 13:02 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-06-08 10:24 [PATCH] ob-sql.el: Support sqlcmd and cygwin environment Xi Shen
2016-06-10 22:06 ` Nicolas Goaziou
2016-06-11 2:17 ` Xi Shen
2016-06-11 8:40 ` Nicolas Goaziou
2016-06-12 2:12 ` Xi Shen
2016-06-12 10:37 ` Xi Shen
2016-06-13 5:36 ` Xi Shen
2016-06-13 6:30 ` Xi Shen
2016-06-14 11:52 ` Nicolas Goaziou
2016-06-14 13:02 ` Xi Shen [this message]
2016-06-15 4:01 ` Xi Shen
2016-06-15 16:49 ` Nicolas Goaziou
2016-06-16 6:04 ` Xi Shen
2016-06-16 8:56 ` tumashu
2016-06-16 22:29 ` Nicolas Goaziou
2016-06-20 12:34 ` Xi Shen
2016-07-04 8:11 ` Xi Shen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.orgmode.org/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CANO68EN+bZ-CV35vd7_h7bBJDY0VK2NNqBdyFwRMEr26Jecz5A@mail.gmail.com \
--to=davidshen84@gmail.com \
--cc=Emacs-orgmode@gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).