emacs-orgmode
[Top][All Lists]
Advanced

[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

Attachment: 0001-ob-C.el-Add-support-for-specifying-namespaces-in-C-C.patch
Description: Text Data


reply via email to

[Prev in Thread] Current Thread [Next in Thread]