emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
From: Maxim Nikulin <manikulin@gmail.com>
To: emacs-orgmode@gnu.org
Subject: Re: bug#47885: [PATCH] org-table-import: Make it more smarter for interactive use
Date: Sat, 5 Jun 2021 19:40:36 +0700	[thread overview]
Message-ID: <s9frc6$htu$1@ciao.gmane.io> (raw)
In-Reply-To: <87o8cmi8bn.fsf@gmail.com>

On 04/06/2021 11:04, Utkarsh Singh wrote:
> On 2021-06-02, 23:44 +0700, Maxim Nikulin wrote:
> 
>> On 02/06/2021 22:08, Utkarsh Singh wrote:
>>> ;;;###autoload
>>>    (defun org-table-import (file separator)
>>> @@ -955,12 +971,13 @@ lines.  It can have the following values:
>>>    - integer When a number, use that many spaces, or a TAB, as field separator.
>>>    - regexp  When a regular expression, use it to match the separator."
>>>      (interactive (list (read-file-name "Import file: ")
>>> -                     (prefix-numeric-value current-prefix-arg)))
>>> +                     current-prefix-arg))
>>
>> It seems, prefix argument works now. Let me remind that file name
>> completion was working better before your change.

If you are still going to update this patch, I recommend you to check if 
`list' form of `interactive' declaration is supported by Emacs 24.3. 
Recently Kyle had to almost revert one my patch. Fortunately a simpler 
alternative supported by old Emacs versions were already discussed.

>> I have noticed a couple of error messages. Unsure what is going wrong, I
>> hope, command to launch emacs is correct (-Q -L ~/src/org-mode/lisp
>> test.org).
>>
>> At startup:
>> Eager macro-expansion failure: (error "rx ‘not’ syntax error: (or 10 44)")
>>
>> In response to M-x org-table-import:
>> rx-check-not: rx ‘not’ syntax error: (or 10 44)
> 
> Is `rx' library loading correctly? If no then you can try:
> 
> +(require 'rx)

This line should be added to org-table.el, but notice "rx ‘not’ syntax 
error: (or ". This logical operators have a bit different meaning for 
regular expression.

Nicolas suggested you to use `list' to avoid `eval'. Certainly `eval' 
should be avoided whenever possible, but there is alternative with "`" 
and "," instead of "'". Though I often prefer `list'.

> 
> Can you also share test.org so that I can test locally?

I can reproduce it with an empty org file. Emacs-26.3. So I am curious 
if your tested your patch at all.

> Now on the topic of CSV parser, I have succesfully implemented a CSV
> parser in less than 65 LOC which also preserves newline character but I
> am facing a dilemma on how should I represent it as a Org table.  For
> ex:
...
> 6,"Cell with new
> Line",6.28

Due to
https://orgmode.org/worg/org-faq.html#Tables
"Will there ever be support for multiple lines in a table field? No."
(there are may be some tricks in Org manual) any variant is acceptable, 
even current one. Since during export split cell is considered as 
different rows, it is responsibility of the user to preprocess original 
file and import it in a form suitable for Org.

However independently of chosen variant, it would be great to get list 
of warnings with ambiguous lines similar to compiler errors. Such 
feature is not implemented in Org yet, the most close is `org-lint' 
result (its behavior differs from compiler errors).

> 2. Represent every newline as distinct cell.  I was trying to implement
> this method but the algorithm that I came up with requires time
> complexity of O(n^2) to just print each row which is too much for large
> CSV files.
> 
> (defun org-table--print-row (row)
>    (let ((maxlines 1)
>          list1 list2 tmp)
>      ;; get maxlines
>      (dolist (i row)

I expect complexity O(number of lines in result). I do not like 
excessive usage of `dolist'. I would try slicing subrows using `mapcar' 
while there is at least one non-nil element.

P.S. Have you read "Structure and Interpretation of Computer Programs" 
by Abelson, Sussman, and Sussman?
https://sarabander.github.io/sicp/html/index.xhtml
or https://mitpress.mit.edu/sites/default/files/sicp/index.html
While reading, it is necessary to realize that elisp is not scheme, tail 
recursion should be avoided in elisp and there are over differences.



  reply	other threads:[~2021-06-05 12:41 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-19  4:43 Utkarsh Singh
2021-04-19  8:19 ` Nicolas Goaziou
2021-04-19 14:23   ` Utkarsh Singh
2021-04-20 13:40     ` Nicolas Goaziou
2021-04-20 17:15       ` Utkarsh Singh
2021-04-23  4:58       ` Utkarsh Singh
2021-04-27 20:21         ` bug#47885: " Nicolas Goaziou
2021-04-28  8:37           ` Utkarsh Singh
2021-04-28 16:38             ` Maxim Nikulin
2021-05-10 18:36               ` Utkarsh Singh
2021-05-12 17:08                 ` Maxim Nikulin
2021-05-14 14:54                   ` Utkarsh Singh
2021-05-15  9:13                     ` Bastien
2021-05-15 10:10                       ` Utkarsh Singh
2021-05-15 10:30                         ` Bastien
2021-05-15 11:09                           ` Utkarsh Singh
2021-05-17  5:29                         ` Bastien
2021-05-17 16:27                           ` Utkarsh Singh
2021-06-01 16:23                           ` Maxim Nikulin
2021-06-01 17:46                             ` Utkarsh Singh
2021-06-02 12:06                               ` Maxim Nikulin
2021-06-02 15:08                                 ` Utkarsh Singh
2021-06-02 16:44                                   ` Maxim Nikulin
2021-06-04  4:04                                     ` Utkarsh Singh
2021-06-05 12:40                                       ` Maxim Nikulin [this message]
2021-06-05 17:50                                         ` Utkarsh Singh
2021-06-09 12:15                                           ` Maxim Nikulin
2021-09-26  8:40                                           ` Bastien
2021-05-16 16:24                     ` Maxim Nikulin
2021-05-17 16:30                       ` Utkarsh Singh
2021-05-18 10:24                       ` Utkarsh Singh
2021-05-18 12:31                         ` Maxim Nikulin
2021-05-18 15:05                           ` Utkarsh Singh

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.orgmode.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='s9frc6$htu$1@ciao.gmane.io' \
    --to=manikulin@gmail.com \
    --cc=emacs-orgmode@gnu.org \
    --subject='Re: bug#47885: [PATCH] org-table-import: Make it more smarter for interactive use' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Code repositories for project(s) associated with this inbox:

	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).