Hi,

On 2021-04-27, 22:21 +0200, Nicolas Goaziou <mail@nicolasgoaziou.fr> wrote:

>> + When using org-table-import interactively if we failed to guess
>> separator then we will be left with a user-error message and an
>> 'unconverted table'.  We can make use of 'temp-buffer' to import our
>> file after successfully conversion.
>
> I'm not sure to understand what you mean.

Note: I will advice you to apply patch no. 2 before trying out the
following example.

1. Download the attached CSV file.  We can call this example.csv

2. Go to *scratch* buffer.

3. Use 'M-x org-table-import' to import example.csv as org-table.

You will see even thought org-table-guess-separator failed in guessing
separator we are still left with unconverted region added to our buffer.

>> + Conversion part of org-table-convert-region make a distinction between
>> '(4) (comma separator) and rest of the separator we should either string
>> version of comma as AND condition or rewrite to simplify it.
>
> Ditto. But it can be the object of another patch. Let's concentrate on
> `org-table-guess-separator' first.
>
>> I am willing to do these possible changes but currently waiting for your
>> review for org-table-guess-separator as there can be more serious bugs
>> lurking around on my code which I am considering base for these
>> changes.
>
> You should definitely write tests for this function. Here's a start:
>
>     (ert-deftest test-org-table/guess-separator ()
>       "Test `test-org-table/guess-separator'."
>       ;; Test space separator.
>       (should
>        (equal " "
>               (org-test-with-temp-text "a b\nc d"
>                 (org-table-guess-separator (point-min) (point-max)))))
>       (should
>        (equal " "
>               (org-test-with-temp-text "a b\nc d"
>                 (org-table-guess-separator (point-min) (point-max)))))
>       ;; Test "inverted" region.
>       (should
>        (equal " "
>               (org-test-with-temp-text "a b\nc d"
>                 (org-table-guess-separator (point-max) (point-min)))))
>       ;; Do not error on empty region.
>       (should-not
>        (org-test-with-temp-text ""
>          (org-table-guess-separator (point-max) (point-min))))
>       (should-not
>        (org-test-with-temp-text "   \n"
>          (org-table-guess-separator (point-max) (point-min)))))
>

I will surely do more testing.

I would also like to simplify the condition for guessing SPACE as
separator due to following cases:

+ field1 'this is field2' 'this is field3' :: In this case we still have
SPACE inside quote (' in this case).

+ Since SPACE is our last valid separator I think searching for a line
which doesn't contains space is more than enough.

Required patch: