[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[nongnu] elpa/subed a99444f666 1/2: Try to avoid sorting already-sorted
|
From: |
ELPA Syncer |
|
Subject: |
[nongnu] elpa/subed a99444f666 1/2: Try to avoid sorting already-sorted buffers |
|
Date: |
Mon, 7 Feb 2022 00:58:47 -0500 (EST) |
branch: elpa/subed
commit a99444f666423f7949e8137b6a43f00a5cef5d56
Author: Sacha Chua <sacha@sachachua.com>
Commit: Sacha Chua <sacha@sachachua.com>
Try to avoid sorting already-sorted buffers
This should reduce the disruption to things like the mark
ring. (Related to #57 and #55, and might address #35 as well?)
* subed/subed-common.el (sanitize): Separate the sanitize and
sanitize-format functions.
(sanitize-format): New generic function.
(validate): Separate the validate and validate-format functions.
(validate-format): New generic function.
(regenerate-ids): Have generic implementation.
(subed-sanitize-functions): Move to subed-config.el.
(subed-validate-functions): Move to subed-config.el.
(subed-prepare-to-save): Call subed-sanitize and subed-validate.
(subed--sorted-p): New function to test if the subtitles are sorted.
(sort): Make interactive. Sort only if unsorted, which should
minimize interference with flycheck and the mark ring.
* subed/subed-config.el (subed-sanitize-functions): New customizable
variable.
(subed-validate-functions): New customizable variable.
* subed/subed-srt.el (subed--sanitize-format): Define this instead of
subed--sanitize.
(subed--validate-format): Define this instead of subed--validate.
* subed/subed-vtt.el (subed--sanitize-format): Define this instead of
subed--sanitize.
(subed--validate-format): Define this instead of subed--validate.
(subed--sort): Remove format-specific implementation.
* tests/test-subed-common.el ("Trimming subtitles when configured to
check on save reports overlaps."): Specify options.
("Sorting"): Add test cases.
* tests/test-subed-vtt.el ("Sorting preserves point in the current
subtitle when subtitle text is empty."): Tweak position check.
---
subed/subed-common.el | 86 +++++++++++++++++++++++++++++-----------------
subed/subed-config.el | 19 ++++++++++
subed/subed-srt.el | 8 ++---
subed/subed-vtt.el | 23 ++-----------
tests/test-subed-common.el | 50 +++++++++++++++++++++++++--
tests/test-subed-srt.el | 24 +++++++------
tests/test-subed-vtt.el | 7 ++--
7 files changed, 142 insertions(+), 75 deletions(-)
diff --git a/subed/subed-common.el b/subed/subed-common.el
index 8896900d19..cbe08c47ee 100644
--- a/subed/subed-common.el
+++ b/subed/subed-common.el
@@ -339,13 +339,26 @@ If BEG and END are not specified, use the whole buffer."
(nreverse result)))
(subed-define-generic-function sanitize ()
- "Remove surplus newlines and whitespace.")
+ "Sanitize this file."
+ (interactive)
+ (run-hooks 'subed-sanitize-functions))
+
+(subed-define-generic-function sanitize-format ()
+ "Remove surplus newlines and whitespace."
+ nil)
(subed-define-generic-function validate ()
- "Move point to the first invalid subtitle and report an error.")
+ "Move point to the first invalid subtitle and report an error."
+ (interactive)
+ (run-hooks 'subed-validate-functions))
+
+(subed-define-generic-function validate-format ()
+ "Validate format-specific rules."
+ nil)
(subed-define-generic-function regenerate-ids ()
- "Ensure consecutive, unduplicated subtitle IDs in formats that use them.")
+ "Ensure consecutive, unduplicated subtitle IDs in formats that use them."
+ nil)
(defvar-local subed--regenerate-ids-soon-timer nil)
(subed-define-generic-function regenerate-ids-soon ()
@@ -1745,42 +1758,51 @@ the stop time isn't smaller than the start time."
;;; Sorting and sanitizing
-(defvar-local subed-sanitize-functions
- '(subed-sort subed-trim-overlap-maybe-sanitize)
- "Functions to sanitize this buffer.
-Functions can clean up whitespace, rearrange subtitles, etc.")
-
-(defvar-local subed-validate-functions
- '(subed-validate subed-trim-overlap-maybe-check)
- "Functions to validate this buffer.
-Validation functions should throw an error or prompt the user for
-action.")
-
(defun subed-prepare-to-save ()
"Sanitize and validate this buffer."
(interactive)
(atomic-change-group
- (run-hooks 'subed-sanitize-functions)
- (run-hooks 'subed-validate-functions)))
+ (subed-sanitize)
+ (subed-validate)))
+
+(defun subed--sorted-p (&optional list)
+ "Return non-nil if LIST is sorted by start time.
+If LIST is nil, use the subtitles in the current buffer."
+ (let ((subtitles (or list (subed-subtitle-list)))
+ (sorted t))
+ (while (cdr subtitles)
+ (if (and
+ (numberp (elt (car subtitles) 1))
+ (numberp (elt (cadr subtitles) 1))
+ (> (elt (car subtitles) 1)
+ (elt (cadr subtitles) 1))) ; starts later than the next one
+ (setq sorted nil
+ subtitles nil)
+ (setq subtitles (cdr subtitles))))
+ sorted))
(subed-define-generic-function sort ()
"Sort subtitles."
- (atomic-change-group
- (subed-sanitize)
- (subed-validate)
- (subed-save-excursion
- (goto-char (point-min))
- (unless (subed-subtitle-id)
- (subed-forward-subtitle-id))
- (sort-subr nil
- ;; nextrecfun (move to next record/subtitle or to end-of-buffer
- ;; if there are no more records)
- (lambda () (unless (subed-forward-subtitle-id)
- (goto-char (point-max))))
- ;; endrecfun (move to end of current record/subtitle)
- #'subed-jump-to-subtitle-end
- ;; startkeyfun (return sort value of current record/subtitle)
- #'subed-subtitle-msecs-start))))
+ (interactive)
+ (subed-sanitize-format)
+ (subed-validate-format)
+ (unless (subed--sorted-p)
+ (subed-batch-edit
+ (atomic-change-group
+ (subed-save-excursion
+ (goto-char (point-min))
+ (unless (subed-jump-to-subtitle-id)
+ (subed-forward-subtitle-id))
+ (sort-subr nil
+ ;; nextrecfun (move to next record/subtitle or to
end-of-buffer
+ ;; if there are no more records)
+ (lambda () (unless (subed-forward-subtitle-id)
+ (goto-char (point-max))))
+ ;; endrecfun (move to end of current record/subtitle)
+ #'subed-jump-to-subtitle-end
+ ;; startkeyfun (return sort value of current
record/subtitle)
+ #'subed-subtitle-msecs-start))
+ (subed-regenerate-ids)))))
;;; Initialization
diff --git a/subed/subed-config.el b/subed/subed-config.el
index 839f13a151..715ad02855 100644
--- a/subed/subed-config.el
+++ b/subed/subed-config.el
@@ -164,6 +164,25 @@ subtitles or negative duration."
:type 'boolean
:group 'subed)
+(defcustom subed-sanitize-functions
+ '(subed-sanitize-format
+ subed-sort
+ subed-trim-overlap-maybe-sanitize)
+ "Functions to call when sanitizing subtitles."
+ :type '(repeat function)
+ :local t
+ :group 'subed)
+
+(defcustom subed-validate-functions
+ '(subed-validate-format
+ subed-trim-overlap-maybe-check)
+ "Functions to validate this buffer.
+Validation functions should throw an error or prompt the user for
+action."
+ :type '(repeat function)
+ :local t
+ :group 'subed)
+
(defcustom subed-loop-seconds-before 1
"Prelude in seconds when looping over subtitle(s)."
:type 'float
diff --git a/subed/subed-srt.el b/subed/subed-srt.el
index 4391307614..3a8a6d4a50 100644
--- a/subed/subed-srt.el
+++ b/subed/subed-srt.el
@@ -314,7 +314,7 @@ Use the format-specific function for MAJOR-MODE."
(insert id-str)))
(setq id (1+ id))))))))
-(cl-defmethod subed--sanitize (&context (major-mode subed-srt-mode))
+(cl-defmethod subed--sanitize-format (&context (major-mode subed-srt-mode))
"Remove surplus newlines and whitespace.
Use the format-specific function for MAJOR-MODE."
(atomic-change-group
@@ -361,7 +361,7 @@ Use the format-specific function for MAJOR-MODE."
(unless (= (length (match-string 0)) 5)
(replace-match " --> "))))))))
-(cl-defmethod subed--validate (&context (major-mode subed-srt-mode))
+(cl-defmethod subed--validate-format (&context (major-mode subed-srt-mode))
"Move point to the first invalid subtitle and report an error.
Use the format-specific function for MAJOR-MODE."
(when (> (buffer-size) 0)
@@ -390,10 +390,6 @@ Use the format-specific function for MAJOR-MODE."
(error "Found invalid stop time: %S" (substring (or
(thing-at-point 'line :no-properties) "\n") 0 -1))))
(goto-char orig-point))))))
-(cl-defmethod subed--sort :after (&context (major-mode subed-srt-mode))
- "Renumber after sorting. Format-specific for MAJOR-MODE."
- (subed-regenerate-ids))
-
(cl-defmethod subed--insert-subtitle :after (&context (major-mode
subed-srt-mode) &optional arg)
"Renumber afterwards. Format-specific for MAJOR-MODE."
(subed-regenerate-ids-soon)
diff --git a/subed/subed-vtt.el b/subed/subed-vtt.el
index 45f7ce8400..908ab8fa03 100644
--- a/subed/subed-vtt.el
+++ b/subed/subed-vtt.el
@@ -285,7 +285,7 @@ Use the format-specific function for MAJOR-MODE."
;;; Maintenance
-(cl-defmethod subed--sanitize (&context (major-mode subed-vtt-mode))
+(cl-defmethod subed--sanitize-format (&context (major-mode subed-vtt-mode))
"Remove surplus newlines and whitespace.
Use the format-specific function for MAJOR-MODE."
(atomic-change-group
@@ -332,7 +332,7 @@ Use the format-specific function for MAJOR-MODE."
(unless (= (length (match-string 0)) 5)
(replace-match " --> "))))))))
-(cl-defmethod subed--validate (&context (major-mode subed-vtt-mode))
+(cl-defmethod subed--validate-format (&context (major-mode subed-vtt-mode))
"Move point to the first invalid subtitle and report an error.
Use the format-specific function for MAJOR-MODE."
(when (> (buffer-size) 0)
@@ -357,25 +357,6 @@ Use the format-specific function for MAJOR-MODE."
(error "Found invalid stop time: %S" (substring (or
(thing-at-point 'line :no-properties) "\n") 0 -1))))
(goto-char orig-point))))))
-(cl-defmethod subed--sort (&context (major-mode subed-vtt-mode))
- "Sanitize, then sort subtitles by start time.
-Use the format-specific function for MAJOR-MODE."
- (atomic-change-group
- (subed-sanitize)
- (subed-validate)
- (subed-save-excursion
- (goto-char (point-min))
- (subed-forward-subtitle-id)
- (sort-subr nil
- ;; nextrecfun (move to next record/subtitle or to end-of-buffer
- ;; if there are no more records)
- (lambda () (unless (subed-forward-subtitle-id)
- (goto-char (point-max))))
- ;; endrecfun (move to end of current record/subtitle)
- #'subed-jump-to-subtitle-end
- ;; startkeyfun (return sort value of current record/subtitle)
- #'subed-subtitle-msecs-start))))
-
;;;###autoload
(define-derived-mode subed-vtt-mode subed-mode "Subed-VTT"
"Major mode for editing WebVTT subtitle files."
diff --git a/tests/test-subed-common.el b/tests/test-subed-common.el
index 5b868dcfb2..df7f29adf2 100644
--- a/tests/test-subed-common.el
+++ b/tests/test-subed-common.el
@@ -3081,16 +3081,20 @@ This is another.
(expect (subed-subtitle-msecs-stop 2) :to-equal 3800)
(expect (subed-subtitle-msecs-stop 3) :to-equal 4800)))))
(describe "when configured to check on save,"
- (it "reports overlaps after sorting."
+ (it "reports overlaps."
(with-temp-srt-buffer
(insert "1\n00:00:01,000 --> 00:00:02,000\nA\n\n"
"2\n00:00:04,000 --> 00:00:06,000\nA\n\n"
"3\n00:00:03,000 --> 00:00:04,500\nA\n\n"
"4\n00:00:05,000 --> 00:00:06,000\nA\n\n")
(let ((subed-trim-overlap-check-on-save t)
+ (subed-trim-overlap-on-save nil)
(subed-subtitle-spacing 200))
+ (spy-on 'subed-trim-overlap-check :and-call-through)
+ (spy-on 'subed-trim-overlaps :and-call-through)
(spy-on 'yes-or-no-p :and-return-value t)
(subed-prepare-to-save)
+ (expect 'subed-trim-overlap-check :to-have-been-called)
(expect 'yes-or-no-p :to-have-been-called)
(expect (subed-subtitle-msecs-stop 1) :to-equal 2000)
(expect (subed-subtitle-msecs-stop 2) :to-equal 3800)
@@ -3130,7 +3134,49 @@ This is another.
'((1 61000 65123 "Foo.")
(2 122234 130345 "Bar.")))))
)
- )
+ (describe "Sorting"
+ (it "detects sorted lists."
+ (expect (subed--sorted-p '((1 1000 2000 "Test")
+ (2 2000 3000 "Test")
+ (3 3000 4000 "Test")))))
+ (it "detects unsorted lists."
+ (expect (subed--sorted-p '((1 3000 2000 "Test")
+ (2 4000 3000 "Test")
+ (3 1000 4000 "Test")))
+ :to-be nil))
+ (it "doesn't happen in an empty buffer."
+ (with-temp-srt-buffer
+ (spy-on 'sort-subr :and-call-through)
+ (subed-sort)
+ (expect 'sort-subr :not :to-have-been-called)))
+ (describe "already-sorted subtitles"
+ (it "doesn't rearrange subtitles."
+ (with-temp-srt-buffer
+ (insert mock-srt-data)
+ (spy-on 'sort-subr :and-call-through)
+ (subed-sort)
+ (expect 'sort-subr :not :to-have-been-called)))
+ (it "maintains the mark ring."
+ (with-temp-srt-buffer
+ (insert mock-srt-data)
+ (let ((mark-ring))
+ (push-mark 10 t nil)
+ (push-mark 20 t nil)
+ (push-mark 3 t nil)
+ (expect (marker-position (car mark-ring)) :to-be 20)
+ (expect (marker-position (cadr mark-ring)) :to-be 10)
+ (subed-sort)
+ (expect (marker-position (car mark-ring)) :to-be 20)
+ (expect (marker-position (cadr mark-ring)) :to-be 10)))))
+ (it "sorts subtitles by start time."
+ (with-temp-srt-buffer
+ (insert mock-srt-data "\n4\n00:02:01,000 --> 00:03:01,000\nNot
sorted.\n")
+ (expect (subed--sorted-p) :to-be nil)
+ (goto-char (point-min))
+ (subed-sort)
+ (expect (subed-subtitle-text 2) :to-equal "Not sorted.")
+ (expect (subed-subtitle-text 3) :to-equal "Bar.")
+ (expect (subed-subtitle-text 4) :to-equal "Baz.")))))
(describe "An old generic function"
:var ((function-list
diff --git a/tests/test-subed-srt.el b/tests/test-subed-srt.el
index 957bb4b13a..553b122b7d 100644
--- a/tests/test-subed-srt.el
+++ b/tests/test-subed-srt.el
@@ -1410,17 +1410,19 @@ Baz.
(expect (buffer-string) :to-equal "")))
(it "runs before saving."
(with-temp-srt-buffer
- (insert mock-srt-data)
- (goto-char (point-min))
- (re-search-forward " --> ")
- (replace-match " --> ")
- (re-search-forward " --> ")
- (replace-match " --> ")
- (re-search-forward " --> ")
- (replace-match "-->")
- (expect (buffer-string) :not :to-equal mock-srt-data)
- (subed-prepare-to-save)
- (expect (buffer-string) :to-equal mock-srt-data))))
+ (insert mock-srt-data)
+ (goto-char (point-min))
+ (re-search-forward " --> ")
+ (replace-match " --> ")
+ (re-search-forward " --> ")
+ (replace-match " --> ")
+ (re-search-forward " --> ")
+ (replace-match "-->")
+ (spy-on 'subed-sanitize :and-call-through)
+ (expect (buffer-string) :not :to-equal mock-srt-data)
+ (subed-prepare-to-save)
+ (expect 'subed-sanitize :to-have-been-called)
+ (expect (buffer-string) :to-equal mock-srt-data))))
(describe "Renumbering"
(it "ensures consecutive subtitle IDs."
diff --git a/tests/test-subed-vtt.el b/tests/test-subed-vtt.el
index 7f27d9eb31..038d519cfd 100644
--- a/tests/test-subed-vtt.el
+++ b/tests/test-subed-vtt.el
@@ -1236,9 +1236,10 @@ Baz.
(it "when subtitle text is empty."
(with-temp-vtt-buffer
(insert "WEBVTT\n\n00:12:01.000 --> 00:01:05.123\n")
- (goto-char (point-max))
- (subed-sort)
- (expect (point) :to-equal (1- (point-max)))))
+ (let ((pos (point)))
+ (subed-sort)
+ (expect (buffer-string) :to-equal "WEBVTT\n\n00:12:01.000 -->
00:01:05.123\n\n")
+ (expect (point) :to-equal pos))))
)
)
(describe "Converting msecs to timestamp"