Hello,
Achim Gratz <strom...@nexgo.de> writes:
> Am 21.03.2013 22:02, schrieb Nicolas Goaziou:
>> I suggest the following code instead, which allows to escape the
>> escaping backslash so the comma is not escaped:
>>
>> (args (mapcar 'org-trim
>> (split-string
>> (replace-regexp-in-string
>> "\\(\\\\+\\)?\\(,\\)"
>> (lambda (str)
>> (let ((slashes (match-string 1 str)))
>> (if (or (not slashes) (evenp (length slashes)))
>> "\\1\000"
>> (concat (make-string (1- (length slashes)) ?\\)
>> ","))))
>> (org-match-string-no-properties 3))
>> "\000")))
>>
>> What do you think about it?
>
> I think this is a lot harder to understand
Actually it is harder to understand because it doesn't make any sense:
the code is wrong. More on this below.
> and I would guess it is also quite a bit slower.
Speed difference is not significant here.
> Also I'm not sure why you are trying to match multiple backslashes.
> The original implementation and the description of the syntax says
> that the only character that can be escaped is a comma, so the new
> implementation changes behaviour in that regard (maybe intentionally,
> I can't tell).
Actually the code I pasted is wrong, I meant:
(split-string
(replace-regexp-in-string
"\\(\\\\+\\)?\\(,\\)"
(lambda (str)
(let ((len (length (match-string 1 str))))
(if (evenp len) (concat (make-string (/ len 2) ?\\) "\000")
(concat (make-string (/ (1- len) 2) ?\\) ","))))
(org-matcĥ-string-no-properties 3) nil t)
"\000")
With the current implementation (and in your refactoring), it is
impossible to have '("a\,b").
If you allow to escape a character, you should also be able to escape
the escaping character. With this patch:
"a,b" -> '("a" "b")
"a\,b" -> '("a,b")
"a\\,b" -> '("a\" "b")
"a\\\,b" -> '("a\,b")
"a\\\\,b" -> '(a"\\" "b")
Note that with the patch, you only need to escape backslashes before
a comma:
"a\\b\,c" -> '("a\\b,c")
If consistency is a matter, a slightly different patch can require to
escape every backslash character. Though, I don't think it is necessary.
Regards,
--
Nicolas Goaziou
