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

[Nicolas Goaziou <mail@nicolasgoaziou.fr>]

Vladimir Panteleev <thecybershadow@gmail.com> writes:

> I wrote about this in the cover letter too. $"foo" and $ "foo" are
> both the same thing.

Just to make it clear: I read the cover letter. My confusion doesn't
come from the fact I may not have read it.

> In both cases, they are two distinct lisp tokens.
> The way $ is presented as a string/reference "prefix" is through
> convention only.
>
> Or are you objecting on stylistic grounds, that the test case from my
> patch doesn't follow the convention of omitting whitespace after the
> $ token, thus making it look like a prefix? This is, of course,
> subjective, but I would prefer to not perpetuate the illusion that
> $"foo" is some magical Emacs Lisp language syntax for a new kind of
> string literal, at least in the test suite.

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?

The same goes for the next string. $"foo", or in your case, $"a\"b\"c"
means nothing in `org-sbe' context. A reference should follow the dollar
character, per `org-sbe' docstring. I suggest to make an equivalent test
with, e.g., $"@1$1", where @1$1 refers to a field containing "a\"b\"c"
or some such.

I suggest to stick to the specifications, which, in this case, are the
docstring, not the code.

> The references are substituted with string literals before the
> $-prefix handling occurs. This is why it doesn't work with ranges.

I understand it doesn't work with ranges, but, per above, your tests are
confusing, IMO.

> I agree that it is all very confusing, and there is a lot of room for
> improvement. That doesn't stand in the way of this patch series,
> though.

I don't think anything stands in the way, but I'd rather get the tests
right, first.

> No difference. As can be seen from org-sbe's implementation, the
> normalization of symbols and string literals occurs before attempting
> to resolve references.

Something is rotten in the state of "ob-table.el", may think.