From: Nicolas Goaziou <email@example.com> To: Utkarsh Singh <firstname.lastname@example.org> Cc: email@example.com Subject: bug#47885: [PATCH] org-table-import: Make it more smarter for interactive use Date: Tue, 27 Apr 2021 22:21:20 +0200 [thread overview] Message-ID: <firstname.lastname@example.org> (raw) In-Reply-To: <email@example.com> (Utkarsh Singh's message of "Fri, 23 Apr 2021 10:28:24 +0530") Hello, Utkarsh Singh <firstname.lastname@example.org> writes: > I am attaching my patch which also include my previous suggestion of > including yes-or-no prompt to org-table-import to allow file which don't > have csv, tsv or txt as extension. I suggest to make several patches. Do not try to stuff as many changes as possible in a single one, please. > + 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. > + 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))))) > + (end (save-excursion > + (goto-char (max beg end0)) This should be beg0 instead of beg above. > + (sep-regexp '(("," (rx bol (1+ (not (or ?\n ?,))) eol)) > + ("\t" (rx bol (1+ (not (or ?\n ?\t))) eol)) > + (";" (rx bol (1+ (not (or ?\n ?\;))) eol)) > + (":" (rx bol (1+ (not (or ?\n ?:))) eol)) > + (" " (rx bol (1+ (not (or ?' ?\" )) > + (not (or ?\s ?\;)) > + (not (or ?' ?\"))) eol)))) Use (sep-regexp (list (list "," (rx bol (1+ (not (or ?\n ?,))) eol)) (list "\t" (rx bol (1+ (not (or ?\n ?\t))) eol)) (list ";" (rx bol (1+ (not (or ?\n ?\;))) eol)) (list ":" (rx bol (1+ (not (or ?\n ?:))) eol)) (list " " (rx bol (1+ (not (or ?' ?\" )) (not (or ?\s ?\;)) (not (or ?' ?\"))) eol)))) so you don't need eval below, and rx forms become constants when compiled. > + sep) This `sep' binding can be removed. > + (unless (= beg end) > + (save-excursion > + (goto-char beg) > + (catch :found > + (pcase-dolist (`(,sep ,regexp) sep-regexp) > + (save-excursion > + (unless (re-search-forward (eval regexp) end t) You can drop the `eval'. > (when (and (called-interactively-p 'any) > - (not (string-match-p (rx "." (or "txt" "tsv" "csv") eos) file))) > + (not (string-match-p (rx "." (or "txt" "tsv" "csv") eos) file)) > + (not (yes-or-no-p "File does not havs .txt .txt .csv as extension. Do you still want to continue? "))) "does not have" and ".txt" -> ".tsv" I guess. Also please provide a patch with a commit message, possibly using `git format-patch'. Thanks! Regards, -- Nicolas Goaziou
next prev parent reply other threads:[~2021-04-27 20:22 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 ` Nicolas Goaziou [this message] 2021-04-28 8:37 ` bug#47885: " 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 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 \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.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).