From mboxrd@z Thu Jan 1 00:00:00 1970 From: Matthew MacLean Subject: Re: fa5fd6351605912ec75e783cb62649 breaks org-babel-script-escape for ob-ruby Date: Wed, 12 Aug 2015 14:05:16 -0600 Message-ID: References: <87h9o5316t.fsf@kyleam.com> <87bned311f.fsf@kyleam.com> <874mk5coqy.fsf@kmlap.domain.org> <87fv3pxdar.fsf@kmlap.domain.org> <87oaic4e3e.fsf@kyleam.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary=089e0115eef8ec18c3051d22bd4d Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:59304) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZPcHL-0007Ob-AM for emacs-orgmode@gnu.org; Wed, 12 Aug 2015 16:05:40 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZPcHJ-0005Z0-AL for emacs-orgmode@gnu.org; Wed, 12 Aug 2015 16:05:38 -0400 Received: from mail-ob0-x231.google.com ([2607:f8b0:4003:c01::231]:36229) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZPcHI-0005YQ-GM for emacs-orgmode@gnu.org; Wed, 12 Aug 2015 16:05:36 -0400 Received: by obnw1 with SMTP id w1so21360639obn.3 for ; Wed, 12 Aug 2015 13:05:36 -0700 (PDT) In-Reply-To: <87oaic4e3e.fsf@kyleam.com> 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-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org To: Kyle Meyer Cc: emacs-orgmode@gnu.org --089e0115eef8ec18c3051d22bd4d Content-Type: multipart/alternative; boundary=089e0115eef8ec18bc051d22bd4b --089e0115eef8ec18bc051d22bd4b Content-Type: text/plain; charset=UTF-8 In that case... Here is another patch with your suggestions. Thanks for taking the time to point out all that..! I'll be sure to keep it all in mind if I submit something else later. On Wed, Aug 12, 2015 at 11:21 AM, Kyle Meyer wrote: > Matthew MacLean writes: > > > Alright, done. Is this acceptable? (Provided that tests don't count > towards > > line count, of course) > > Thanks. A few minor comments on the commit message. > > > Subject: [PATCH] ob-ruby: Fix double-escaping > > > > * lisp/ob-ruby.el: Remove second call to > > `org-babel-ruby-table-or-string' in `org-babel-ruby-evaluate'. > > Please add the name of the changed function in parentheses after the > file name rather than putting it in the description body. > > > * testing/lisp/test-ob-ruby.el: Add test to verify > > `org-babel-execute:ruby' can evaluate Ruby code. (What the > > double-escape prevented) > > Same here for the test name. "Add test." for description would do. > > > I removed the escaping from `org-babel-ruby-evaluate', because the only > > place `org-babel-ruby-evaluate' is ever called is in > > `org-babel-execute:ruby'. > > > > In this function, its result either escaped (Where the double escape > > previously occurred) or passed in as the "scalar-form" of > > `org-babel-result-cond', which handles the "pp" and "code" parameters. > > (A place that doesn't need escaping.) > > I think the above two paragraphs could be replaced by a link to this ML > post. > > Thanks for working on this. > > -- > Kyle > --089e0115eef8ec18bc051d22bd4b Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
In that case... Here is another patch with your sugge= stions.

Thanks for taking the time to point out all that..! I'll= be sure to keep it all in mind if I submit something else later.
=

On Wed, Aug= 12, 2015 at 11:21 AM, Kyle Meyer <kyle@kyleam.com> wrote:
=
Matthew MacLean <archenoth@gmail.com> writes:

> Alright, done. Is this acceptable? (Provided that tests don't coun= t towards
> line count, of course)

Thanks.=C2=A0 A few minor comments on the commit message.

> Subject: [PATCH] ob-ruby: Fix double-escaping
>
> * lisp/ob-ruby.el: Remove second call to
>=C2=A0 =C2=A0`org-babel-ruby-table-or-string' in `org-babel-ruby-ev= aluate'.

Please add the name of the changed function in parentheses after the
file name rather than putting it in the description body.

> * testing/lisp/test-ob-ruby.el: Add test to verify
>=C2=A0 =C2=A0`org-babel-execute:ruby' can evaluate Ruby code. (What= the
>=C2=A0 =C2=A0double-escape prevented)

Same here for the test name.=C2=A0 "Add test." for description wo= uld do.

> I removed the escaping from `org-babel-ruby-evaluate', because the= only
> place `org-babel-ruby-evaluate' is ever called is in
> `org-babel-execute:ruby'.
>
> In this function, its result either escaped (Where the double escape > previously occurred) or passed in as the "scalar-form" of > `org-babel-result-cond', which handles the "pp" and &quo= t;code" parameters.
> (A place that doesn't need escaping.)

I think the above two paragraphs could be replaced by a link to this ML
post.

Thanks for working on this.

--
Kyle

--089e0115eef8ec18bc051d22bd4b-- --089e0115eef8ec18c3051d22bd4d Content-Type: text/x-patch; charset=US-ASCII; name="0001-ob-ruby-Fix-double-escaping.patch" Content-Disposition: attachment; filename="0001-ob-ruby-Fix-double-escaping.patch" Content-Transfer-Encoding: base64 X-Attachment-Id: f_id96t01w0 RnJvbSAxNzQ0NzMxOWVlZTUzZmJmY2M2MTIzYzM4NDg0MmRkN2ZkMDRlM2I4IE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBBcmNoZW5vdGggPGFyY2hlbm90aEBnbWFpbC5jb20+CkRhdGU6 IFR1ZSwgMTEgQXVnIDIwMTUgMjM6NTk6MjUgLTA2MDAKU3ViamVjdDogW1BBVENIXSBvYi1ydWJ5 OiBGaXggZG91YmxlLWVzY2FwaW5nCgoqIGxpc3Avb2ItcnVieS5lbCAob3JnLWJhYmVsLXJ1Ynkt ZXZhbHVhdGUpOiBSZW1vdmUgc2Vjb25kIGNhbGwgdG8KICBgb3JnLWJhYmVsLXJ1YnktdGFibGUt b3Itc3RyaW5nJy4KCiogdGVzdGluZy9saXNwL3Rlc3Qtb2ItcnVieS5lbCAob3JnLWJhYmVsLXJ1 YnktZXZhbHVhdGUpOiBBZGQgdGVzdC4KCjxodHRwOi8vcGVybWFsaW5rLmdtYW5lLm9yZy9nbWFu ZS5lbWFjcy5vcmdtb2RlLzk5ODg4PgoKVElOWUNIQU5HRQotLS0KIGxpc3Avb2ItcnVieS5lbCAg ICAgICAgICAgICAgfCAgNiArLS0tLS0KIHRlc3RpbmcvbGlzcC90ZXN0LW9iLXJ1YnkuZWwgfCAx NyArKysrKysrKysrKysrKysrKwogMiBmaWxlcyBjaGFuZ2VkLCAxOCBpbnNlcnRpb25zKCspLCA1 IGRlbGV0aW9ucygtKQoKZGlmZiAtLWdpdCBhL2xpc3Avb2ItcnVieS5lbCBiL2xpc3Avb2ItcnVi eS5lbAppbmRleCA5YjAxZGJmLi4wZmY0NjBlIDEwMDY0NAotLS0gYS9saXNwL29iLXJ1YnkuZWwK KysrIGIvbGlzcC9vYi1ydWJ5LmVsCkBAIC0yMDEsMTEgKzIwMSw3IEBAIHJldHVybiB0aGUgdmFs dWUgb2YgdGhlIGxhc3Qgc3RhdGVtZW50IGluIEJPRFksIGFzIGVsaXNwLiIKIAkJCSAgICAgIG9y Zy1iYWJlbC1ydWJ5LXBwLXdyYXBwZXItbWV0aG9kCiAJCQkgICAgb3JnLWJhYmVsLXJ1Ynktd3Jh cHBlci1tZXRob2QpCiAJCQkgIGJvZHkgKG9yZy1iYWJlbC1wcm9jZXNzLWZpbGUtbmFtZSB0bXAt ZmlsZSAnbm9xdW90ZSkpKQotCQkgKGxldCAoKHJhdyAob3JnLWJhYmVsLWV2YWwtcmVhZC1maWxl IHRtcC1maWxlKSkpCi0gICAgICAgICAgICAgICAgICAgKGlmIChvciAobWVtYmVyICJjb2RlIiBy ZXN1bHQtcGFyYW1zKQotICAgICAgICAgICAgICAgICAgICAgICAgICAgKG1lbWJlciAicHAiIHJl c3VsdC1wYXJhbXMpKQotICAgICAgICAgICAgICAgICAgICAgICByYXcKLSAgICAgICAgICAgICAg ICAgICAgIChvcmctYmFiZWwtcnVieS10YWJsZS1vci1zdHJpbmcgcmF3KSkpKSkpCisJCSAob3Jn LWJhYmVsLWV2YWwtcmVhZC1maWxlIHRtcC1maWxlKSkpKQogICAgIDs7IGNvbWludCBzZXNzaW9u IGV2YWx1YXRpb24KICAgICAoY2FzZSByZXN1bHQtdHlwZQogICAgICAgKG91dHB1dApkaWZmIC0t Z2l0IGEvdGVzdGluZy9saXNwL3Rlc3Qtb2ItcnVieS5lbCBiL3Rlc3RpbmcvbGlzcC90ZXN0LW9i LXJ1YnkuZWwKaW5kZXggZWI1MjMzYi4uNTc2Y2IxMyAxMDA2NDQKLS0tIGEvdGVzdGluZy9saXNw L3Rlc3Qtb2ItcnVieS5lbAorKysgYi90ZXN0aW5nL2xpc3AvdGVzdC1vYi1ydWJ5LmVsCkBAIC0y MSw2ICsyMSwyMyBAQAogKHVubGVzcyAoZmVhdHVyZXAgJ29iLXJ1YnkpCiAgIChzaWduYWwgJ21p c3NpbmctdGVzdC1kZXBlbmRlbmN5ICJTdXBwb3J0IGZvciBSdWJ5IGNvZGUgYmxvY2tzIikpCiAK KyhlcnQtZGVmdGVzdCB0ZXN0LW9iLXJ1YnkvYmFzaWMtZXZhbHVhdGlvbiAoKQorICAiVGVzdCB0 aGF0IGJhc2ljIGV2YWx1YXRpb24gd29ya3MuIgorICAgIChzaG91bGQgKGVxdWFsIChvcmctdGVz dC13aXRoLXRlbXAtdGV4dCAiIytiZWdpbl9zcmMgcnVieQorIDIgKyAyCisjK2VuZF9zcmMiCisg IChvcmctYmFiZWwtZXhlY3V0ZS1tYXliZSkKKyAgKHN1YnN0cmluZy1uby1wcm9wZXJ0aWVzCisg ICAoYnVmZmVyLXN0cmluZykpKQorCQkgICAiIytiZWdpbl9zcmMgcnVieQorIDIgKyAyCisjK2Vu ZF9zcmMKKworIytSRVNVTFRTOgorOiA0CisKKyIpKSkKKwogKGVydC1kZWZ0ZXN0IHRlc3Qtb2It cnVieS9zZXNzaW9uLW91dHB1dC0xICgpCiAgICAgKHNob3VsZCAoZXF1YWwgKG9yZy10ZXN0LXdp dGgtdGVtcC10ZXh0ICIjK2JlZ2luX3NyYyBydWJ5IDpzZXNzaW9uIDpyZXN1bHRzIG91dHB1dAog cyA9IFwiMVwiCi0tIAoyLjUuMAoK --089e0115eef8ec18c3051d22bd4d--