[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [O] [PATCH] Support for 'using namespace *' in ob-C.el
From: |
Jay Kamat |
Subject: |
Re: [O] [PATCH] Support for 'using namespace *' in ob-C.el |
Date: |
Mon, 31 Jul 2017 01:03:53 -0700 |
Hello!
Sorry for the late reply, I was pretty busy last week. An updated
patch is attached!
> I suggest to add the following to "ob-C.el" so that `org-lint' can issue
> a warning whenever :namespaces is used in a C block.
Done. I needed to tweak the code a bit though to get it to work and to
prevent flagging valid C headers as errors with org lint.
> Using `org-entry-get' is no longer supported. You can replace the `or'
> with
>
> (cdr (assq :namespaces params))
Done, I also replaced the other uses of 'org-entry-get' around the one
I modified
> Nitpick: I would put the "\n" on another line.
I agree, and done :)
> Could you also provide an ORG-NEWS entry for the feature?
Done. I'm not sure if it went in the right place and if it's formatted
correctly though, so can you give that a look over to make sure it
looks good?
Thanks again for taking the time to review this for me :D
Let me know if you spot anything fishy or wrong.
Also, RE: Copyright, this is the form I need to send, correct?
http://orgmode.org/request-assign-future.txt
-Jay
On Sun, Jul 23, 2017 at 7:44 AM, Nicolas Goaziou <address@hidden> wrote:
> Hello,
>
> Jay Kamat <address@hidden> writes:
>
>> However, it would be nice to add a "using namespace std" to this
>> source code block, so it can become:
>>
>> #+BEGIN_SRC C++ :includes <iostream> :namespaces std
>> cout << "Hello world\n";
>> #+END_SRC
>>
>>
>> Which makes it cleaner and easier to read, especially for very short
>> code snippets, using a bunch of std tools.
>
> Good idea.
>
>> One concern that I have is that "using namespace *;" is only available
>> in C++ and not C, but there isn't an easy way I could find to limit
>> it's usage to only C++ blocks without a bunch of restructuring, so
>> this will fail if you attempt to set a namespace on a plain C block.
>
> I suggest to add the following to "ob-C.el" so that `org-lint' can issue
> a warning whenever :namespaces is used in a C block.
>
> (defconst org-babel-header-args:C '((includes . :any))
> "C-specific header arguments.")
>
> (defconst org-babel-header-args:C++
> `(,(append '((namespaces . :any))
> org-babel-header-args:C))
> "C++-specific header arguments.")
>
>> Also, this contribution puts me very close to the 15 line limit before
>> I need to get FSF papers signed. I intend to sign papers soon, but I'm
>> a little busy right now, and I'll get around to submitting the request
>> later on.
>
> Thank you. FYI, in many cases, the whole process is very quick and not
> time-consuming.
>
>> Subject: [PATCH] ob-C.el: Add support for specifying namespaces in C/C++
>>
>> * lisp/ob-C.el (org-babel-C-expand-C): Add a :namespaces export option
>> to C++ org babel blocks. Namespaces specified here will be added to
>> the file in the format 'using namespace %s;'. Multiple namespaces
>> can be specified, separated by spaces.
>
> Some comments follow.
>
>> + (namespaces (org-babel-read
>> + (or (cdr (assq :namespaces params))
>> + (org-entry-get nil "namespaces" t))
>> + nil)))
>
> Using `org-entry-get' is no longer supported. You can replace the `or'
> with
>
> (cdr (assq :namespaces params))
>
>> + ;; namespaces
>> + (mapconcat
>> + (lambda (inc) (format "using namespace %s;" inc))
>> + namespaces "\n")
>
> Nitpick: I would put the "\n" on another line.
>
> Could you also provide an ORG-NEWS entry for the feature?
>
> Regards,
>
> --
> Nicolas Goaziou
0001-ob-C.el-Add-support-for-specifying-namespaces-in-C-C.patch
Description: Text Data