Re: [O] [PATCH 1/3] ob-table: Fix org-sbe's handling of quotes in arguments

[Nicolas Goaziou]

Hello,

Vladimir Panteleev <address@hidden> writes:

> On 2018-03-14 15:00, Nicolas Goaziou wrote:

>> I disagree. You are testing an implementation detail here: the fact that
>> "$" is not necessarily a prefix. According to the docstring, it should
>> be, so the test should use that, too. What if we rewrite `org-sbe' at
>> some point?
>
> I'm sorry if I didn't explain it properly in my previous messages.
> I'll try again.
>
> The fact that $"foo" and $ "foo" mean the same thing is not an
> implementation detail of org-sbe. It is a consequence of Emacs Lisp
> grammar.

[...]

> So, whether the test case has a space between $ and "a\"b\"c" is as
> relevant as whether it has comments, or uses tabs instead of spaces
> for indentation.
>
> I hope this explanation can put this issue to rest.

We're clearly mis-communicating. I know the difference between a symbol
and a string, and how `read' operates. I think what puzzles me is some
design choices made in `org-sbe', and the fact that the second note of
its docstring is clear as mud.

> org-sbe's docstring is misleading:

I agree.

> $ is not a way to quote just table references, but any string literals
> in general. Had it been otherwise (i.e. $-prefixing being part of the
> table reference syntax), there would be no way to pass a string
> literal (which isn't an interpolated table cell value) to the
> indicated function. The doc string should probably be improved in this
> regard.

I consider it to be a bug if you need to write $"string" instead of
"string" in any `org-sbe' call. We should not test such a mis-feature,
which should be removed.

> Adding a test which uses a table reference instead of a string literal
> won't hurt, but it would be testing several layers at once, and,
> assuming cell value interpolation into emacs lisp table formulas is
> already tested somewhere else, superfluous.

Let me insist on this. $"string" (or $ "string") is very wrong. Please
do not write your tests on top of this. IIUC, `org-sbe' is about passing
table fields into source blocks. Let's test that instead, with a table,
and a source block. I don't mind superfluous coverage.

It is just a matter of tests. Your changes sound good. So, would you
mind adjusting your tests so we can move forward?

Regards,

-- 
Nicolas Goaziou