emacs-languagetool / flymake-languagetool

Flymake support for LanguageTool
GNU General Public License v3.0
33 stars 11 forks source link

Allow flymake-languagetool to skip errors of certain faces #13

Closed Inc0n closed 1 year ago

Inc0n commented 1 year ago

Address this issue: https://github.com/emacs-languagetool/flymake-languagetool/issues/12

Inspired by https://github.com/cjl8zf/langtool-ignore-fonts.

It filters out error at positions (.offset) if the face text-property at point is part of the specified faces to ignore. It is customized per major mode.

Inc0n commented 1 year ago

Ok, all suggestiones are now applied.

Inc0n commented 1 year ago

During my usage so far, I run into this problem which prompts:

Buffer " http localhost:8081-186939" has a running process; kill it? (y or n) n

I think this is caused by my usage of with-current-buffer inside the check-all function. It may be best moved this filtering elsewhere, e.g. flymake-languagetool--handle-finished, which is how it is done in langtool-ignore-fonts.

tpeacock19 commented 1 year ago

can you move that inside your check function flymake-languagetool-ignore-at-pos-p?

Inc0n commented 1 year ago

can you move that inside your check function flymake-languagetool-ignore-at-pos-p?

I applied this change, so now faces are fetched this way (get-text-property pos 'face src-buf), however, the issue still persist.

This makes me think that if filtering must be done outside of flymake-languagetool--check-all, this feature could alternativelly just live outside of this package, as an addon, similar to langtool-ignore-fonts (done using advices).

tpeacock19 commented 1 year ago

Can you push the latest commits you have so I can check as well?

Inc0n commented 1 year ago

Sure I will get that done.

Inc0n commented 1 year ago

Ok comitted, here is a more complete summary of the issue (the last one miss some details)

During my usage so far, I run into this problem which prompts:

Buffer " http localhost:8081-186939" has a running process; kill it? (y or n) n

This happens, when I am saving the edited the source buffer where flymake-languagetool is enabled. The step that sometimes trigger this issue, (I had not found a consistent reproduceable steps)

  1. Edit the source buffer
  2. Save it
  3. The promp may appear Note: certain text edits trigger this issue more consistently.

I think this is because we are accessing the buffer for face text property information inside flymake-languagetool-check-all. Last report suggested with-current-buffer may be the cause, this is inaccurate now, as the issue persist in the latest commit, which uses an alternative way to access buffer information by passing source buffer into get-text-property.

Inc0n commented 1 year ago

Ahh, after inspecting the backtrace with debug-on-error as t. It seems this was not caused by the changes in my commits.

I was using the flymake-languagetool-20220922, which is why there was the unintentional deleted in my first commit.

The top parts of the backtrace ```lisp Debugger entered--Lisp error: (error "[Flymake] Obsolete report from backend flymake-lan...") signal(error ("[Flymake] Obsolete report from backend flymake-lan...")) error("[Flymake] Obsolete report from backend flymake-lan...") flymake-error("Obsolete report from backend %s with explanation %..." flymake-languagetool--checker nil) flymake--handle-report(flymake-languagetool--checker backend-token5698 (#s(flymake--diag :locus # :beg 9585 :end 9587 :type :warning :text "Replace by the typographical symbol. [LanguageTool..." :backend nil :data ((suggestions "±")) :overlay-properties nil :overlay nil :orig-beg 9585 :orig-end 9587) #s(flymake--diag :locus # :beg 8993 :end 9002 :type :warning :text "Uncountable nouns are usually not used with an ind..." :backend nil :data ((suggestions "freedom")) :overlay-properties nil :overlay nil :orig-beg 8993 :orig-end 9002) #s(flymake--diag :locus # :beg 7830 :end 7835 :type :warning :text "Possible typo detected. [LanguageTool]" :backend nil :data ((suggestions "inch" "inn" "Inca" "ICN" "incl" "INC" "Inc" "inc" "incs" "IUCN" "Incan")) :overlay-properties nil :overlay nil :orig-beg 7830 :orig-end 7835) #s(flymake--diag :locus # :beg 5681 :end 5686 :type :warning :text "The verb ‘set up’ is spelled as two words. The nou..." :backend nil :data ((suggestions "set up")) :overlay-properties nil :overlay nil :orig-beg 5681 :orig-end 5686) #s(flymake--diag :locus # :beg 4956 :end 4961 :type :warning :text "Insert a space between the numerical value and the..." :backend nil :data ((suggestions "0.8 cm")) :overlay-properties nil :overlay nil :orig-beg 4956 :orig-end 4961) #s(flymake--diag :locus # :beg 4480 :end 4488 :type :warning :text "The verb ‘help’ is used with an infinitive. [Langu..." :backend nil :data ((suggestions "to manage" "manage")) :overlay-properties nil :overlay nil :orig-beg 4480 :orig-end 4488) #s(flymake--diag :locus # :beg 2732 :end 2734 :type :warning :text "Put a space after the comma, but not before the co..." :backend nil :data ((suggestions ",")) :overlay-properties nil :overlay nil :orig-beg 2732 :orig-end 2734) #s(flymake--diag :locus # :beg 2423 :end 2425 :type :warning :text "Replace by the typographical symbol. [LanguageTool..." :backend nil :data ((suggestions "±")) :overlay-properties nil :overlay nil :orig-beg 2423 :orig-end 2425) #s(flymake--diag :locus # :beg 2388 :end 2390 :type :warning :text "Replace by the typographical symbol. [LanguageTool..." :backend nil :data ((suggestions "±")) :overlay-properties nil :overlay nil :orig-beg 2388 :orig-end 2390) #s(flymake--diag :locus # :beg 1827 :end 1831 :type :warning :text "Consider using a more common abbreviation of this ..." :backend nil :data ((suggestions "B.Eng.")) :overlay-properties nil :overlay nil :orig-beg 1827 :orig-end 1831) #s(flymake--diag :locus # :beg 558 :end 562 :type :warning :text "Consider using a more common abbreviation of this ..." :backend nil :data ((suggestions "B.Eng.")) :overlay-properties nil :overlay nil :orig-beg 558 :orig-end 562) #s(flymake--diag :locus # :beg 414 :end 416 :type :warning :text "Don’t put a space before the closing parenthesis. ..." :backend nil :data ((suggestions "}")) :overlay-properties nil :overlay nil :orig-beg 414 :orig-end 416) #s(flymake--diag :locus # :beg 26 :end 33 :type :warning :text "Possible typo detected. [LanguageTool]" :backend nil :data ((suggestions "paper" "a paper")) :overlay-properties nil :overlay nil :orig-beg 26 :orig-end 33)) :region (1 . 9907)) apply(flymake--handle-report flymake-languagetool--checker backend-token5698 ((#s(flymake--diag :locus # :beg 9585 :end 9587 :type :warning :text "Replace by the typographical symbol. [LanguageTool..." :backend nil :data ((suggestions "±")) :overlay-properties nil :overlay nil :orig-beg 9585 :orig-end 9587) #s(flymake--diag :locus # :beg 8993 :end 9002 :type :warning :text "Uncountable nouns are usually not used with an ind..." :backend nil :data ((suggestions "freedom")) :overlay-properties nil :overlay nil :orig-beg 8993 :orig-end 9002) #s(flymake--diag :locus # :beg 7830 :end 7835 :type :warning :text "Possible typo detected. [LanguageTool]" :backend nil :data ((suggestions "inch" "inn" "Inca" "ICN" "incl" "INC" "Inc" "inc" "incs" "IUCN" "Incan")) :overlay-properties nil :overlay nil :orig-beg 7830 :orig-end 7835) #s(flymake--diag :locus # :beg 5681 :end 5686 :type :warning :text "The verb ‘set up’ is spelled as two words. The nou..." :backend nil :data ((suggestions "set up")) :overlay-properties nil :overlay nil :orig-beg 5681 :orig-end 5686) #s(flymake--diag :locus # :beg 4956 :end 4961 :type :warning :text "Insert a space between the numerical value and the..." :backend nil :data ((suggestions "0.8 cm")) :overlay-properties nil :overlay nil :orig-beg 4956 :orig-end 4961) #s(flymake--diag :locus # :beg 4480 :end 4488 :type :warning :text "The verb ‘help’ is used with an infinitive. [Langu..." :backend nil :data ((suggestions "to manage" "manage")) :overlay-properties nil :overlay nil :orig-beg 4480 :orig-end 4488) #s(flymake--diag :locus # :beg 2732 :end 2734 :type :warning :text "Put a space after the comma, but not before the co..." :backend nil :data ((suggestions ",")) :overlay-properties nil :overlay nil :orig-beg 2732 :orig-end 2734) #s(flymake--diag :locus # :beg 2423 :end 2425 :type :warning :text "Replace by the typographical symbol. [LanguageTool..." :backend nil :data ((suggestions "±")) :overlay-properties nil :overlay nil :orig-beg 2423 :orig-end 2425) #s(flymake--diag :locus # :beg 2388 :end 2390 :type :warning :text "Replace by the typographical symbol. [LanguageTool..." :backend nil :data ((suggestions "±")) :overlay-properties nil :overlay nil :orig-beg 2388 :orig-end 2390) #s(flymake--diag :locus # :beg 1827 :end 1831 :type :warning :text "Consider using a more common abbreviation of this ..." :backend nil :data ((suggestions "B.Eng.")) :overlay-properties nil :overlay nil :orig-beg 1827 :orig-end 1831) #s(flymake--diag :locus # :beg 558 :end 562 :type :warning :text "Consider using a more common abbreviation of this ..." :backend nil :data ((suggestions "B.Eng.")) :overlay-properties nil :overlay nil :orig-beg 558 :orig-end 562) #s(flymake--diag :locus # :beg 414 :end 416 :type :warning :text "Don’t put a space before the closing parenthesis. ..." :backend nil :data ((suggestions "}")) :overlay-properties nil :overlay nil :orig-beg 414 :orig-end 416) #s(flymake--diag :locus # :beg 26 :end 33 :type :warning :text "Possible typo detected. [LanguageTool]" :backend nil :data ((suggestions "paper" "a paper")) :overlay-properties nil :overlay nil :orig-beg 26 :orig-end 33)) :region (1 . 9907))) #f(compiled-function (&rest args) #)((#s(flymake--diag :locus # :beg 9585 :end 9587 :type :warning :text "Replace by the typographical symbol. [LanguageTool..." :backend nil :data ((suggestions "±")) :overlay-properties nil :overlay nil :orig-beg 9585 :orig-end 9587) #s(flymake--diag :locus # :beg 8993 :end 9002 :type :warning :text "Uncountable nouns are usually not used with an ind..." :backend nil :data ((suggestions "freedom")) :overlay-properties nil :overlay nil :orig-beg 8993 :orig-end 9002) #s(flymake--diag :locus # :beg 7830 :end 7835 :type :warning :text "Possible typo detected. [LanguageTool]" :backend nil :data ((suggestions "inch" "inn" "Inca" "ICN" "incl" "INC" "Inc" "inc" "incs" "IUCN" "Incan")) :overlay-properties nil :overlay nil :orig-beg 7830 :orig-end 7835) #s(flymake--diag :locus # :beg 5681 :end 5686 :type :warning :text "The verb ‘set up’ is spelled as two words. The nou..." :backend nil :data ((suggestions "set up")) :overlay-properties nil :overlay nil :orig-beg 5681 :orig-end 5686) #s(flymake--diag :locus # :beg 4956 :end 4961 :type :warning :text "Insert a space between the numerical value and the..." :backend nil :data ((suggestions "0.8 cm")) :overlay-properties nil :overlay nil :orig-beg 4956 :orig-end 4961) #s(flymake--diag :locus # :beg 4480 :end 4488 :type :warning :text "The verb ‘help’ is used with an infinitive. [Langu..." :backend nil :data ((suggestions "to manage" "manage")) :overlay-properties nil :overlay nil :orig-beg 4480 :orig-end 4488) #s(flymake--diag :locus # :beg 2732 :end 2734 :type :warning :text "Put a space after the comma, but not before the co..." :backend nil :data ((suggestions ",")) :overlay-properties nil :overlay nil :orig-beg 2732 :orig-end 2734) #s(flymake--diag :locus # :beg 2423 :end 2425 :type :warning :text "Replace by the typographical symbol. [LanguageTool..." :backend nil :data ((suggestions "±")) :overlay-properties nil :overlay nil :orig-beg 2423 :orig-end 2425) #s(flymake--diag :locus # :beg 2388 :end 2390 :type :warning :text "Replace by the typographical symbol. [LanguageTool..." :backend nil :data ((suggestions "±")) :overlay-properties nil :overlay nil :orig-beg 2388 :orig-end 2390) #s(flymake--diag :locus # :beg 1827 :end 1831 :type :warning :text "Consider using a more common abbreviation of this ..." :backend nil :data ((suggestions "B.Eng.")) :overlay-properties nil :overlay nil :orig-beg 1827 :orig-end 1831) #s(flymake--diag :locus # :beg 558 :end 562 :type :warning :text "Consider using a more common abbreviation of this ..." :backend nil :data ((suggestions "B.Eng.")) :overlay-properties nil :overlay nil :orig-beg 558 :orig-end 562) #s(flymake--diag :locus # :beg 414 :end 416 :type :warning :text "Don’t put a space before the closing parenthesis. ..." :backend nil :data ((suggestions "}")) :overlay-properties nil :overlay nil :orig-beg 414 :orig-end 416) #s(flymake--diag :locus # :beg 26 :end 33 :type :warning :text "Possible typo detected. [LanguageTool]" :backend nil :data ((suggestions "paper" "a paper")) :overlay-properties nil :overlay nil :orig-beg 26 :orig-end 33)) :region (1 . 9907)) funcall(#f(compiled-function (&rest args) #) (#s(flymake--diag :locus # :beg 9585 :end 9587 :type :warning :text "Replace by the typographical symbol. [LanguageTool..." :backend nil :data ((suggestions "±")) :overlay-properties nil :overlay nil :orig-beg 9585 :orig-end 9587) #s(flymake--diag :locus # :beg 8993 :end 9002 :type :warning :text "Uncountable nouns are usually not used with an ind..." :backend nil :data ((suggestions "freedom")) :overlay-properties nil :overlay nil :orig-beg 8993 :orig-end 9002) #s(flymake--diag :locus # :beg 7830 :end 7835 :type :warning :text "Possible typo detected. [LanguageTool]" :backend nil :data ((suggestions "inch" "inn" "Inca" "ICN" "incl" "INC" "Inc" "inc" "incs" "IUCN" "Incan")) :overlay-properties nil :overlay nil :orig-beg 7830 :orig-end 7835) #s(flymake--diag :locus # :beg 5681 :end 5686 :type :warning :text "The verb ‘set up’ is spelled as two words. The nou..." :backend nil :data ((suggestions "set up")) :overlay-properties nil :overlay nil :orig-beg 5681 :orig-end 5686) #s(flymake--diag :locus # :beg 4956 :end 4961 :type :warning :text "Insert a space between the numerical value and the..." :backend nil :data ((suggestions "0.8 cm")) :overlay-properties nil :overlay nil :orig-beg 4956 :orig-end 4961) #s(flymake--diag :locus # :beg 4480 :end 4488 :type :warning :text "The verb ‘help’ is used with an infinitive. [Langu..." :backend nil :data ((suggestions "to manage" "manage")) :overlay-properties nil :overlay nil :orig-beg 4480 :orig-end 4488) #s(flymake--diag :locus # :beg 2732 :end 2734 :type :warning :text "Put a space after the comma, but not before the co..." :backend nil :data ((suggestions ",")) :overlay-properties nil :overlay nil :orig-beg 2732 :orig-end 2734) #s(flymake--diag :locus # :beg 2423 :end 2425 :type :warning :text "Replace by the typographical symbol. [LanguageTool..." :backend nil :data ((suggestions "±")) :overlay-properties nil :overlay nil :orig-beg 2423 :orig-end 2425) #s(flymake--diag :locus # :beg 2388 :end 2390 :type :warning :text "Replace by the typographical symbol. [LanguageTool..." :backend nil :data ((suggestions "±")) :overlay-properties nil :overlay nil :orig-beg 2388 :orig-end 2390) #s(flymake--diag :locus # :beg 1827 :end 1831 :type :warning :text "Consider using a more common abbreviation of this ..." :backend nil :data ((suggestions "B.Eng.")) :overlay-properties nil :overlay nil :orig-beg 1827 :orig-end 1831) #s(flymake--diag :locus # :beg 558 :end 562 :type :warning :text "Consider using a more common abbreviation of this ..." :backend nil :data ((suggestions "B.Eng.")) :overlay-properties nil :overlay nil :orig-beg 558 :orig-end 562) #s(flymake--diag :locus # :beg 414 :end 416 :type :warning :text "Don’t put a space before the closing parenthesis. ..." :backend nil :data ((suggestions "}")) :overlay-properties nil :overlay nil :orig-beg 414 :orig-end 416) #s(flymake--diag :locus # :beg 26 :end 33 :type :warning :text "Possible typo detected. [LanguageTool]" :backend nil :data ((suggestions "paper" "a paper")) :overlay-properties nil :overlay nil :orig-beg 26 :orig-end 33)) :region (1 . 9907)) (save-current-buffer (set-buffer source-buffer) (funcall report-fn (flymake-languagetool--output-to-errors output source-buffer) :region (cons (point-min) (point-max))) (kill-buffer flymake-languagetool--proc-buf) (setq flymake-languagetool--proc-buf nil)) (let ((output (save-restriction (set-buffer-multibyte t) (goto-char url-http-end-of-headers) (buffer-substring (point) (point-max))))) (save-current-buffer (set-buffer source-buffer) (funcall report-fn (flymake-languagetool--output-to-errors output source-buffer) :region (cons (point-min) (point-max))) (kill-buffer flymake-languagetool--proc-buf) (setq flymake-languagetool--proc-buf nil))) (if (and url-http-end-of-headers (equal (current-buffer) (save-current-buffer (set-buffer source-buffer) flymake-languagetool--proc-buf))) (let ((output (save-restriction (set-buffer-multibyte t) (goto-char url-http-end-of-headers) (buffer-substring (point) (point-max))))) (save-current-buffer (set-buffer source-buffer) (funcall report-fn (flymake-languagetool--output-to-errors output source-buffer) :region (cons (point-min) (point-max))) (kill-buffer flymake-languagetool--proc-buf) (setq flymake-languagetool--proc-buf nil))) (save-current-buffer (set-buffer source-buffer) (flymake--log-1 :warning 'flymake-languagetool "Canceling tha obsolete check %s" flymake-languagetool--proc-buf))) flymake-languagetool--handle-finished(nil # #f(compiled-function (&rest args) #)) apply(flymake-languagetool--handle-finished (nil # #f(compiled-function (&rest args) #))) url-http-activate-callback() url-http-content-length-after-change-function(8188 12097 3909) url-http-generic-filter(#> "https://www.merriam-webster.com/dictionary/set%20u...") read-from-minibuffer("Buffer \" *http localhost:8081*-136701\" has a runni..." nil (keymap (keymap (escape . abort-recursive-edit) (remap keymap (quit . abort-recursive-edit) (exit-prefix . abort-recursive-edit) (exit . y-or-n-p-insert-other) (scroll-other-window-down . minibuffer-scroll-other-window-down) (scroll-other-window . minibuffer-scroll-other-window) (scroll-down . minibuffer-scroll-down-command) (scroll-up . minibuffer-scroll-up-command) (recenter . minibuffer-recenter-top-bottom) (self-insert-command . y-or-n-p-insert-other) (ignore . y-or-n-p-insert-other) (delete-and-edit . y-or-n-p-insert-other) (edit-replacement . y-or-n-p-insert-other) (edit . y-or-n-p-insert-other) (undo-all . y-or-n-p-insert-other) (undo . y-or-n-p-insert-other) (backup . y-or-n-p-insert-other) (skip . y-or-n-p-insert-n) (automatic . y-or-n-p-insert-y) (act-and-exit . y-or-n-p-insert-y) (act-and-show . y-or-n-p-insert-y) (act . y-or-n-p-insert-y)) keymap (menu-bar keymap (Terminal "Terminal" keymap "Terminal" (Terminal\ Buffers menu-item "Terminal Buffers" (keymap "Terminal Buffers"))) (minibuf "Minibuf" keymap (previous menu-item "Previous History Item" previous-history-element :help "Put previous minibuffer history element in the min...") (next menu-item "Next History Item" next-history-element :help "Put next minibuffer history element in the minibuf...") (isearch-backward menu-item "Isearch History Backward" isearch-backward :help "Incrementally search minibuffer history backward") (isearch-forward menu-item "Isearch History Forward" isearch-forward :help "Incrementally search minibuffer history forward") (return menu-item "Enter" exit-minibuffer :key-sequence "\15" :help "Terminate input and exit minibuffer") (quit menu-item "Quit" abort-recursive-edit :help "Abort input and exit minibuffer") "Minibuf")) (13 . exit-minibuffer) (10 . exit-minibuffer) (7 . minibuffer-keyboard-quit) (C-tab . file-cache-minibuffer-complete) (9 . self-insert-command) (XF86Back . previous-history-element) (up . previous-line-or-history-element) (prior . previous-history-element) (XF86Forward . next-history-element) (down . next-line-or-history-element) (next . next-history-element) (27 keymap (63 . session-minibuffer-history-help) (60 . minibuffer-beginning-of-buffer) (114 . previous-matching-history-element) (115 . next-matching-history-element) (112 . previous-history-element) (110 . next-history-element))) keymap (escape . exit-prefix) (M-prior . scroll-other-window-down) (M-next . scroll-other-window) (prior . scroll-down) (next . scroll-up) (27 keymap (33554454 . scroll-other-window-down) (22 . scroll-other-window) (118 . scroll-down)) (22 . scroll-up) (29 . quit) (7 . quit) (63 . help) (help . help) (f1 . help) (8 . help) (85 . undo-all) (117 . undo) (94 . backup) (33 . automatic) (12 . recenter) (23 . delete-and-edit) (18 . edit) (46 . act-and-exit) (return . exit) (13 . exit) (113 . exit) (44 . act-and-show) (69 . edit-replacement) (101 . edit-replacement) (78 . skip) (89 . act) (110 . skip) (121 . act) (backspace . skip) (delete . skip) (127 . skip) (32 . act)) nil empty-history) yes-or-no-p("Buffer \" *http localhost:8081*-136701\" has a runni...") process-kill-buffer-query-function() kill-buffer(#) (progn (flymake--log-1 :warning 'flymake-languagetool "Canceling the obsolete check %s" flymake-languagetool--proc-buf) (if (get-buffer-process flymake-languagetool--proc-buf) (progn (delete-process (get-buffer-process flymake-languagetool--proc-buf)))) (kill-buffer flymake-languagetool--proc-buf) (setq flymake-languagetool--proc-buf nil)) (if (buffer-live-p flymake-languagetool--proc-buf) (progn (flymake--log-1 :warning 'flymake-languagetool "Canceling the obsolete check %s" flymake-languagetool--proc-buf) (if (get-buffer-process flymake-languagetool--proc-buf) (progn (delete-process (get-buffer-process flymake-languagetool--proc-buf)))) (kill-buffer flymake-languagetool--proc-buf) (setq flymake-languagetool--proc-buf nil))) ```
tpeacock19 commented 1 year ago

Okay, you'll need to rebase this on the latest commit.

Also, the lookup of associated faces for the major mode can all be moved into the ignore-at-pos function. something like this:

(with-current-buffer src-buf
  (when-let ((faces-to-ignore (alist-get major-mode flymake-languagetool-ignore-faces-alist))
             (x (get-text-property pos 'face)))
    (cl-intersection faces-to-ignore (ensure-list x))))

Have you done an FSF copyright assignment?

Inc0n commented 1 year ago

Would that not mean, the look up will be done as many times as there is in the errors list?

Yes, I have done the assignment :)

On Sun, 27 Nov 2022, 23:57 tpeacock19, @.***> wrote:

Okay, you'll need to rebase this on the latest commit.

Also, the lookup of associated faces for the major mode can all be moved into the ignore-at-pos function. something like this:

(with-current-buffer src-buf (when-let ((faces-to-ignore (alist-get major-mode flymake-languagetool-ignore-faces-alist)) (x (get-text-property pos 'face))) (cl-intersection faces-to-ignore (ensure-list x))))

Have you done a copyright assignment https://www.gnu.org/software/emacs/manual/html_node/emacs/Copyright-Assignment.html ?

— Reply to this email directly, view it on GitHub https://github.com/emacs-languagetool/flymake-languagetool/pull/13#issuecomment-1328373679, or unsubscribe https://github.com/notifications/unsubscribe-auth/AIDID377XPX44VKKNYBZAE3WKPYOFANCNFSM6AAAAAASMQ4MKA . You are receiving this because you authored the thread.Message ID: @.***>

Inc0n commented 1 year ago

I tried rebasing, it tells me

Current branch master is up to date.

tpeacock19 commented 1 year ago

You're right, it would. I'll have a look after the merge, otherwise this looks good. Thanks for the work!