From: Maxim Nikulin <manikulin@gmail.com> To: emacs-orgmode@gnu.org Subject: Re: [Patch] to correctly sort the items with emphasis marks in a list Date: Wed, 21 Apr 2021 20:10:50 +0700 [thread overview] Message-ID: <s5p88r$go9$1@ciao.gmane.io> (raw) In-Reply-To: <874kg0ae0k.fsf@nicolasgoaziou.fr> [-- Attachment #1: Type: text/plain, Size: 1739 bytes --] On 21/04/2021 03:37, Nicolas Goaziou wrote: > Maxim Nikulin writes: > >> (let ((s (org-sort-remove-invisible >> "A wrapping [[https://orgmode.org?a=b&c=d#e][link]] emphasis/" >> >> I expect "A wrapping link emphasis". > > Yet, your expectations are wrong. There is no link in the text above. > Emphasized text starts at "/wrapping" and ends before "/?". > > Granted, this is a situation where the Org syntax is not very intuitive. > Anyway, the new function is more accurate. I think, new variant should be committed even it does not fix Juan's case. He have not confirmed the fix yet. > Maybe we should require a space after punctuation following emphasized > text. I don't know. This is orthogonal to the current discussion. I still believe in my expectation, however I admit such limitation of parser. At first I have not recognized that the issue may be similar to https://orgmode.org/list/CAH+Wm4-_XHUZKFTf=ZtbfnCPvQWkbEoeGs8EpYm+8SPmu8LHFg@mail.gmail.com/ Anyway for my example workaround is to add more markers before, inside, and after the link. Maybe I will look closer at Tom's parser if it solves ambiguity in the same way. >> In the meanwhile I have tried >> >> (benchmark-run 1 (org-sort-list t ?a)) >> >> in a file (1100 lines) obtained using > > I don't think performance is really an issue. Of course, the suggested > function is clearly slower than the current one. It is OK since difference is not really huge, especially taking into account that new variant was not compiled. Do you still have problem with locale dependency of added tests? I can not guess what could be its source and expect that test should work reliably. Disregard "/3" in the subject of the patches. Third change is your code. [-- Attachment #2: 0001-More-tests-for-org-sort-list.patch --] [-- Type: text/x-patch, Size: 4913 bytes --] From c2c46d133e80ffa2323618ac88a1902e63ba1efc Mon Sep 17 00:00:00 2001 From: Max Nikulin <manikulin@gmail.com> Date: Mon, 19 Apr 2021 19:06:36 +0700 Subject: [PATCH 1/3] More tests for `org-sort-list' * lisp/org.el (org-verbatim-re): Add to the docstring a statement concerning subgroups meaning. * testing/lisp/test-org-list.el (test-org-list/sort): Add cases with text emphasis. Stress in comments that it is significant whether locale-specific collation rules ignores spaces. * testing/lisp/test-org.el (test-org/org-sort-remove-invisible): Add tests for `org-sort-list' helper. --- lisp/org.el | 3 ++- testing/lisp/test-org-list.el | 34 +++++++++++++++++++++++++++++++++- testing/lisp/test-org.el | 25 +++++++++++++++++++++++++ 3 files changed, 60 insertions(+), 2 deletions(-) diff --git a/lisp/org.el b/lisp/org.el index c2738100f..3d4f5553a 100644 --- a/lisp/org.el +++ b/lisp/org.el @@ -3685,7 +3685,8 @@ After a match, the match groups contain these elements: 5 The character after the match, empty at the end of a line") (defvar org-verbatim-re nil - "Regular expression for matching verbatim text.") + "Regular expression for matching verbatim text. +Groups 1-5 have the same meaning as in `org-emph-re' pattern.") (defvar org-emphasis-regexp-components) ; defined just below (defvar org-emphasis-alist) ; defined just below diff --git a/testing/lisp/test-org-list.el b/testing/lisp/test-org-list.el index 3689a172f..cc7914c8d 100644 --- a/testing/lisp/test-org-list.el +++ b/testing/lisp/test-org-list.el @@ -1225,7 +1225,39 @@ b. Item 2<point>" (equal "- b\n- a\n- C\n" (org-test-with-temp-text "- b\n- C\n- a\n" (org-sort-list t ?A) - (buffer-string)))))) + (buffer-string)))) + ;; Emphasis in the beginning. + (should + (equal "- a\n- /z/\n" + (org-test-with-temp-text "- /z/\n- a\n" + (org-sort-list t ?a) + (buffer-string)))) + (should + (equal "- *a*\n- z\n" + (org-test-with-temp-text "- z\n- *a*\n" + (org-sort-list t ?a) + (buffer-string)))) + ;; Emphasis of second word. + ;; Locales with significant spaces (C, es_ES, pl_PL) + ;; are more sensitive to problems with sort. + (should + (equal "- a b\n- a /z/\n" + (org-test-with-temp-text "- a /z/\n- a b\n" + (org-sort-list t ?a) + (buffer-string)))) + (should + (equal "- a *b*\n- a z\n" + (org-test-with-temp-text "- a z\n- a *b*\n" + (org-sort-list t ?a) + (buffer-string)))) + ;; Space role in sorting. + ;; Test would fail for locales with ignored space, e.g. en_US, it works + ;; in C and currently rare locales having significant space (es_ES, pl_PL) + (should + (equal "- Time stamp\n- Timer\n" + (org-test-with-temp-text "- Timer\n- Time stamp\n" + (org-sort-list t ?a) + (buffer-string)))))) ;; Sort numerically. (should (equal "- 1\n- 2\n- 10\n" diff --git a/testing/lisp/test-org.el b/testing/lisp/test-org.el index f6fb4b3ca..3f74b3f35 100644 --- a/testing/lisp/test-org.el +++ b/testing/lisp/test-org.el @@ -8262,4 +8262,29 @@ two (provide 'test-org) +(ert-deftest test-org/org-sort-remove-invisible () + "Empasis markers are stripped by `org-sort-remove-invisible'." + (dolist (case '(("Test *bold* text" . "Test bold text") + ("Test /italic/ text" . "Test italic text") + ("Test _underlined_ text" . "Test underlined text") + ("Test +strike through+ text" . "Test strike through text") + ("Test =verbatim= text" . "Test verbatim text") + ("Test ~code~ text" . "Test code text") + ("*Beginning* of a line" . "Beginning of a line") + ("End =of line=" . "End of line") + ("Braces (*around*)" . "Braces (around)") + ("Multiple *emphasis* options /in the/ same *line*" . + "Multiple emphasis options in the same line") + ("/Adjucent/ *emphasis*" . "Adjucent emphasis") + ("Verbatim =with *emphasis* example=" . + "Verbatim with *emphasis* example") + ("Emphasis [[http://orgmode.org/][inside /a link/]]" . + "Emphasis inside a link") + ("*Wrap [[https:/orgmode.org][link]] emphasis*" . + "Wrap link emphasis") + ("A =verbatim [[https://orgmode.org/][link]] example=" . + "A verbatim [[https://orgmode.org/][link]] example"))) + (let ((test-string (car case)) + (expectation (cdr case))) + (should (equal expectation (org-sort-remove-invisible test-string)))))) ;;; test-org.el ends here -- 2.25.1 [-- Attachment #3: 0002-testing-lisp-test-org.el-Non-obvious-cases-for-org-s.patch --] [-- Type: text/x-patch, Size: 2330 bytes --] From e7a326fff98fdc56b91818af526d398d8387bda4 Mon Sep 17 00:00:00 2001 From: Max Nikulin <manikulin@gmail.com> Date: Wed, 21 Apr 2021 19:22:18 +0700 Subject: [PATCH 2/3] testing/lisp/test-org.el: Non-obvious cases for org-sort-remove-invisible testing/lisp/test-org.el (test-org/org-sort-remove-invisible): Add test cases illustrating corner cases in parser behavior. Fontification in Emacs buffer is not consistent with result of `org-sort-remove-invisible` for 2 of 3 added examples. The intention is to make author of changes in parser aware of consequences, however it is unlikely that users rely on such nuances. Update expectations if change is considered as improvement. --- testing/lisp/test-org.el | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/testing/lisp/test-org.el b/testing/lisp/test-org.el index 3f74b3f35..4f987f509 100644 --- a/testing/lisp/test-org.el +++ b/testing/lisp/test-org.el @@ -8283,7 +8283,18 @@ two ("*Wrap [[https:/orgmode.org][link]] emphasis*" . "Wrap link emphasis") ("A =verbatim [[https://orgmode.org/][link]] example=" . - "A verbatim [[https://orgmode.org/][link]] example"))) + "A verbatim [[https://orgmode.org/][link]] example") + ;; A bit strange cases to catch changes of behavior. + ;; It may be reasonable to update expectation in the case of failure. + ("/Overlapping [[http://orgmode.org][structure/ with link]]" . + ;; not "Overlapping structure with link" + "Overlapping [[http://orgmode.org][structure with link]]") + ("Another [[http://orgmode.org][overlapping *structure]] with* link" . + ;; not "Another overlapping structure with link" + "Another overlapping *structure with* link") + ("A /wrapping [[https://orgmode.org/?bot=true][link]] emphasis/" . + ;; not "A wrapping link emphasis", lost "/" before "?" instead + "A wrapping [[https://orgmode.org?bot=true][link]] emphasis/"))) (let ((test-string (car case)) (expectation (cdr case))) (should (equal expectation (org-sort-remove-invisible test-string)))))) -- 2.25.1
next prev parent reply other threads:[~2021-04-21 13:11 UTC|newest] Thread overview: 54+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-04-02 18:15 Juan Manuel Macías 2021-04-09 22:28 ` Nicolas Goaziou 2021-04-10 0:01 ` Juan Manuel Macías 2021-04-10 10:19 ` Nicolas Goaziou 2021-04-10 11:41 ` Juan Manuel Macías 2021-04-13 17:31 ` Maxim Nikulin 2021-04-13 19:08 ` Juan Manuel Macías 2021-04-14 15:42 ` Maxim Nikulin 2021-04-14 15:51 ` Maxim Nikulin 2021-04-14 17:07 ` Juan Manuel Macías 2021-04-14 21:36 ` Juan Manuel Macías 2021-04-15 14:58 ` Maxim Nikulin 2021-04-15 18:21 ` Juan Manuel Macías 2021-04-16 14:59 ` Maxim Nikulin 2021-04-16 15:30 ` Maxim Nikulin 2021-04-17 13:27 ` Maxim Nikulin 2021-04-18 17:52 ` Juan Manuel Macías 2021-04-18 21:20 ` Juan Manuel Macías 2021-04-19 8:33 ` Nicolas Goaziou 2021-04-19 12:34 ` Maxim Nikulin 2021-04-19 16:08 ` Nicolas Goaziou 2021-04-19 17:00 ` Greg Minshall 2021-04-19 17:17 ` Tom Gillespie 2021-04-19 18:00 ` Greg Minshall 2021-04-19 17:36 ` Maxim Nikulin 2021-04-19 17:50 ` Nicolas Goaziou 2021-04-20 12:37 ` Maxim Nikulin 2021-04-20 12:20 ` Maxim Nikulin 2021-04-20 13:57 ` Nicolas Goaziou 2021-04-20 15:51 ` Maxim Nikulin 2021-04-20 20:37 ` Nicolas Goaziou 2021-04-21 13:10 ` Maxim Nikulin [this message] 2021-04-21 15:45 ` Juan Manuel Macías 2021-04-24 14:41 ` Maxim Nikulin 2021-05-20 17:06 ` [Patch] tests for org-remove-invisible Maxim Nikulin 2021-05-20 18:06 ` Nicolas Goaziou 2021-09-27 16:53 ` Max Nikulin 2021-11-25 12:11 ` [Patch] to correctly sort the items with emphasis marks in a list Ihor Radchenko 2021-11-25 16:59 ` Max Nikulin 2021-05-15 20:43 ` Bastien 2021-05-15 22:09 ` Nicolas Goaziou 2021-05-16 6:04 ` Bastien 2021-04-28 5:46 ` Bastien 2021-04-28 6:37 ` Nicolas Goaziou 2021-04-28 6:49 ` Bastien 2021-04-28 8:04 ` Bastien 2021-05-15 13:32 ` Bastien 2021-05-15 15:10 ` Maxim Nikulin 2021-05-15 20:44 ` Bastien 2021-04-12 13:50 Juan Manuel Macías [not found] <mailman.57.1618243212.17744.emacs-orgmode@gnu.org> 2021-04-12 18:51 ` Ypo 2021-04-12 23:18 ` Juan Manuel Macías 2021-04-12 23:52 ` Samuel Wales 2021-04-13 14:16 ` Juan Manuel Macías
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='s5p88r$go9$1@ciao.gmane.io' \ --to=manikulin@gmail.com \ --cc=emacs-orgmode@gnu.org \ --subject='Re: [Patch] to correctly sort the items with emphasis marks in a list' \ /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).