From 113ccbf3802209f5767235fee4659c94fe4b3220 Mon Sep 17 00:00:00 2001 From: Loic Lemaitre Date: Wed, 6 Mar 2024 00:23:18 +0100 Subject: [PATCH] Improvements on JSX comment algorithm. --- jtsx.el | 163 +++++++++++++++++++++++++++----------------- tests/jtsx-tests.el | 103 +++++++++++++++++++++++++++- 2 files changed, 200 insertions(+), 66 deletions(-) diff --git a/jtsx.el b/jtsx.el index 673ddc5..b74a1a3 100644 --- a/jtsx.el +++ b/jtsx.el @@ -119,6 +119,15 @@ See `treesit-font-lock-level' for more informations." :safe 'booleanp :group 'jtsx) +(defconst jtsx-jsx-ts-keys '("jsx_expression" + "jsx_element" + "jsx_attribute" + "jsx_self_closing_element" + "jsx_text" + "jsx_opening_element" + "jsx_closing_element" + "jsx_namespace_name")) + (defconst jtsx-jsx-ts-root-keys '("jsx_element" "jsx_self_closing_element" "jsx_expression" @@ -140,30 +149,71 @@ See `treesit-font-lock-level' for more informations." "Check if last command has modified the buffer." (< jtsx-last-buffer-chars-modifed-tick (buffer-chars-modified-tick))) +(defun jtsx-treesit-node-at (pos) + "Return the treesit node at POS or POS-1 depending on the context. +Treesit looks at the node after the position (excepted when at the end of the +line), it fine in most situations, excepted when at the end of a region where +getting the treesit node before position is more suitable." + (let ((effective-pos (if (and (region-active-p) (eq pos (region-end))) (1- pos) pos))) + (treesit-node-at effective-pos))) + +(defun jtsx-traversing-jsx-expression-p (node initial-node) + "Check whether we are going outside a jsx expression. +NODE is the current node, and INITIAL-NODE is the node which the research has +started from. We consider that we are not traversing the jsx-expression if +one of the INITIAL-NODE and NODE positions matches (start or end), ie we were +already outside it." + (and (equal (treesit-node-type node) "jsx_expression") + (not (eq (treesit-node-start initial-node) (treesit-node-start node))) + (not (eq (treesit-node-end initial-node) (treesit-node-end node))))) + +(defun jtsx-enclosing-jsx-node (node types &optional fallback-types include-node jsx-exp-guard) + "Get first parent of NODE matching one of TYPES. +If the research failed and FALLBACK-TYPES are not nil retry with FALLBACK-TYPES. +If INCLUDE-NODE is not nil, NODE is included in the research. +If JSX-EXP-GUARD is not nil, do not traverse jsx expression." + (let* ((get-parent-node (lambda (current-node) + (let ((parent-node (treesit-node-parent current-node))) + (if (and jsx-exp-guard + (jtsx-traversing-jsx-expression-p parent-node node)) + nil + parent-node)))) + (enclosing-node (if include-node node (funcall get-parent-node node)))) + (while (and enclosing-node (not (member (treesit-node-type enclosing-node) types))) + (setq enclosing-node (funcall get-parent-node enclosing-node))) + (if (or enclosing-node (not fallback-types)) + enclosing-node + (jtsx-enclosing-jsx-node node fallback-types nil include-node jsx-exp-guard)))) + +(defun jtsx-enclosing-jsx-element (node &optional jsx-exp-guard) + "Get first parent of NODE matching `jsx_element' type. +If JSX-EXP-GUARD is not nil, do not traverse jsx expression." + (jtsx-enclosing-jsx-node node '("jsx_element") nil t jsx-exp-guard)) + +(defun jtsx-enclosing-jsx-element-at-point (&optional jsx-exp-guard) + "Get first parent matching `jsx_element' type at point. +If JSX-EXP-GUARD is not nil, do not traverse jsx expression." + (jtsx-enclosing-jsx-element (jtsx-treesit-node-at (point)) jsx-exp-guard)) + (defun jtsx-node-jsx-context-p (node) - "Check if NODE inside JSX context." - (member (treesit-node-type node) '("jsx_expression" - "jsx_element" - "jsx_attribute" - "jsx_self_closing_element" - "jsx_text" - "jsx_opening_element" - "jsx_closing_element" - "jsx_namespace_name"))) - -(defun jtsx-jsx-context-at-p (position) - "Check if inside JSX context at POSITION." - (save-excursion - (let ((pred (lambda (n) (jtsx-node-jsx-context-p n)))) - (treesit-parent-until (treesit-node-at position) pred t)))) - -(defun jtsx-jsx-context-p () - "Check if in JSX context at point or at region ends." - (and (jtsx-jsx-context-at-p (point)) (or (not (region-active-p)) (jtsx-jsx-context-at-p (mark))))) + "Check if NODE is JSX." + (member (treesit-node-type node) jtsx-jsx-ts-keys)) + +(defun jtsx-jsx-context-at-p (position &optional jsx-exp-guard) + "Check if inside JSX context at POSITION. +If JSX-EXP-GUARD is not nil, do not traverse jsx expression." + (when-let ((node (jtsx-treesit-node-at position))) + (jtsx-enclosing-jsx-node node jtsx-jsx-ts-keys nil t jsx-exp-guard))) + +(defun jtsx-jsx-context-p (&optional jsx-exp-guard) + "Check if in JSX context at point or at region ends. +If JSX-EXP-GUARD is not nil, do not traverse jsx expression." + (and (jtsx-jsx-context-at-p (point) jsx-exp-guard) + (or (not (region-active-p)) (jtsx-jsx-context-at-p (mark) jsx-exp-guard)))) (defun jtsx-jsx-attribute-context-at-p (position) "Check if insinde a JSX element attribute at POSITION." - (when-let ((node (treesit-node-at position))) + (when-let ((node (jtsx-treesit-node-at position))) (jtsx-enclosing-jsx-node node '("jsx_attribute") nil nil t))) (defun jtsx-jsx-attribute-context-p () @@ -184,8 +234,8 @@ See `comment-dwim' documentation for ARG usage." (let ((comment-start "{/* ") (comment-end " */}") (comment-use-syntax nil) - (comment-start-skip "\\([[:space:]]*\\)\\(//+\\|{?/\\*+\\)") - (comment-end-skip "\\*+/}?[[:space:]]*")) + (comment-start-skip "\\(?:{?/\\*+\\)\\s-*") + (comment-end-skip "\\s-*\\(\\*+/}?\\)")) (comment-dwim arg))) (defun jtsx-comment-jsx-attribute-dwim (arg) @@ -194,8 +244,17 @@ See `comment-dwim' documentation for ARG usage." (let ((comment-start "/* ") (comment-end " */") (comment-use-syntax nil) - (comment-start-skip "\\([[:space:]]*\\)\\(//+\\|/\\*+\\)") - (comment-end-skip "\\*+/[[:space:]]*")) + (comment-start-skip "\\(?:/\\*+\\)\\s-*") + (comment-end-skip "\\s-*\\(\\*+/\\)")) + (comment-dwim arg))) + +(defun jtsx-comment-js-nested-in-jsx-dwim (arg) + "Comment or uncomment JSX at point or in region. +See `comment-dwim' documentation for ARG usage." + (let ((comment-use-syntax nil) + (comment-start-skip "\\(?://+\\|{?/\\*+\\)\\s-*") + (comment-end-skip "\\s-*\\(\\s>\\|\\*+/}?\\)")) + (message "comment js-jsx") (comment-dwim arg))) (defun jtsx-comment-dwim (arg) @@ -208,37 +267,16 @@ See `comment-dwim' documentation for ARG usage." (and (not (region-active-p)) (jtsx-jsx-attribute-context-at-p (line-end-position)))) (jtsx-comment-jsx-attribute-dwim arg)) ;; Inside JSX context ? - ((or (and (region-active-p) (jtsx-jsx-context-p)) + ((or (and (region-active-p) (jtsx-jsx-context-p t)) (and (not (region-active-p)) (jtsx-jsx-context-at-p (line-end-position)))) (jtsx-comment-jsx-dwim arg)) + ;; Inside JS nested in JSX context ? + ((or (and (region-active-p) (jtsx-jsx-context-p)) + (and (not (region-active-p)) (jtsx-jsx-context-at-p (line-end-position)))) + (jtsx-comment-js-nested-in-jsx-dwim arg)) ;; General case (t (comment-dwim arg)))) -(defun jtsx-enclosing-jsx-node (node types &optional fallback-types include-node jsx-exp-guard) - "Get first parent of NODE matching one of TYPES. -If the research failed and FALLBACK-TYPES are not nil retry with FALLBACK-TYPES. -If INCLUDE-NODE is not nil, NODE is included in the research. -If JSX-EXP-GUARD is not nil, do not traverse jsx expression." - (let ((enclosing-node (if include-node node (treesit-node-parent node)))) - (while (and enclosing-node (not (member (treesit-node-type enclosing-node) types))) - (setq enclosing-node (if (and jsx-exp-guard - (equal (treesit-node-type enclosing-node) "jsx_expression")) - nil ; give up the research - (treesit-node-parent enclosing-node)))) - (if (or enclosing-node (not fallback-types)) - enclosing-node - (jtsx-enclosing-jsx-node node fallback-types nil include-node jsx-exp-guard)))) - -(defun jtsx-enclosing-jsx-element (node &optional jsx-exp-guard) - "Get first parent of NODE matching `jsx_element' type. -If JSX-EXP-GUARD is not nil, do not traverse jsx expression." - (jtsx-enclosing-jsx-node node '("jsx_element") nil t jsx-exp-guard)) - -(defun jtsx-enclosing-jsx-element-at-point (&optional jsx-exp-guard) - "Get first parent matching `jsx_element' type at point. -If JSX-EXP-GUARD is not nil, do not traverse jsx expression." - (jtsx-enclosing-jsx-element (treesit-node-at (point)) jsx-exp-guard)) - (defun jtsx-jump-jsx-opening-tag () "Jump to the opening tag of the JSX element." (interactive "^") @@ -312,7 +350,7 @@ CHILD-FIELD-NAME identify the tag to rename (`open_tag' or `close_tag')." ;; tree. ;; Note that treesit parser is robust enough to be not too confused by mismaching ;; element tag identifiers. - (let* ((node (treesit-node-at (point))) + (let* ((node (jtsx-treesit-node-at (point))) ;; Field names can be wrong because of mismatching element tag identifiers, so ;; using types is safer here. (current-tag-node-type (treesit-node-type (treesit-node-parent node))) @@ -337,7 +375,7 @@ CHILD-FIELD-NAME identify the tag to rename (`open_tag' or `close_tag')." "Rename a JSX element to NEW-NAME at point. Point can be in the opening or closing." (interactive "sRename element to: ") - (let* ((node (treesit-node-at (point))) + (let* ((node (jtsx-treesit-node-at (point))) (parent-node (treesit-node-parent node)) (parent-node-type (treesit-node-type parent-node))) (unless (and (member (treesit-node-type node) '("identifier" ">")) @@ -397,7 +435,7 @@ Under some circumtances, the synchronization can fail. Right after editing a tree-sitter parser can be lost. For example in that situation, where `A' has just been erased form the opening tag: < attribute=\"text\">." (when (and jtsx-enable-jsx-element-tags-auto-sync (jtsx-command-modified-buffer-p)) - (let* ((node (treesit-node-at (point))) + (let* ((node (jtsx-treesit-node-at (point))) (parent-node (treesit-node-parent node)) (parent-node-type (treesit-node-type parent-node))) (when (and (member (treesit-node-type node) '("identifier" ">" "<")) @@ -467,7 +505,7 @@ If ALLOW-STEP-IN is not nil, a move can go deeper in the JSX hierarchy. Only used if FULL-ELEMENT-MOVE is t. Return a plist containing the move information : `:node-start', `:node-end', `:new-pos'." - (let* ((current-node (jtsx-enclosing-jsx-node (treesit-node-at (point)) + (let* ((current-node (jtsx-enclosing-jsx-node (jtsx-treesit-node-at (point)) jtsx-jsx-ts-element-tag-keys jtsx-jsx-ts-root-keys t @@ -642,7 +680,7 @@ N is a numeric prefix argument. If greater than 1, insert N times `>', but (insert-char (char-from-name "GREATER-THAN SIGN") n t) ;; Check jsx context inside the tag (when (and (= n 1) jtsx-enable-jsx-electric-closing-element (jtsx-jsx-context-at-p (1- (point)))) - (when-let ((node (treesit-node-at (1- (point))))) ; Safer to get node inside the tag + (when-let ((node (jtsx-treesit-node-at (1- (point))))) ; Safer to get node inside the tag (when-let ((parent-node (treesit-node-parent node))) (when (and (equal (treesit-node-type node) ">") (equal (treesit-node-type parent-node) "jsx_opening_element")) @@ -669,7 +707,7 @@ For example: attribute >" (when (jtsx-jsx-context-p) - (when-let* ((node (treesit-node-at (point)))) + (when-let* ((node (jtsx-treesit-node-at (point)))) (when (equal (treesit-node-type node) "{/* Hello */});") + (result "(Hello);")) + (should (equal (comment-dwim-into-buffer content set-region #'jtsx-jsx-mode) result)) + (should (equal (comment-dwim-into-buffer content set-region #'jtsx-tsx-mode) result)))) + (ert-deftest jtsx-test-comment-jsx-attribute-region () (let ((set-region #'(lambda () (find-and-set-region "disabled={false}"))) (content "(Hello);") @@ -230,6 +244,13 @@ Turn this buffer in MODE mode if supplied or defaults to jtsx-tsx-mode." (should (equal (comment-dwim-into-buffer content set-region #'jtsx-jsx-mode) result)) (should (equal (comment-dwim-into-buffer content set-region #'jtsx-tsx-mode) result)))) +(ert-deftest jtsx-test-uncomment-jsx-attribute-region () + (let ((set-region #'(lambda () (find-and-set-region "\\/\\* disabled={false} \\*\\/"))) + (content "(Hello);") + (result "(Hello);")) + (should (equal (comment-dwim-into-buffer content set-region #'jtsx-jsx-mode) result)) + (should (equal (comment-dwim-into-buffer content set-region #'jtsx-tsx-mode) result)))) + (ert-deftest jtsx-test-comment-jsx-boolean-attribute-region () (let ((set-region #'(lambda () (find-and-set-region "disabled"))) (content "(Hello);") @@ -237,6 +258,13 @@ Turn this buffer in MODE mode if supplied or defaults to jtsx-tsx-mode." (should (equal (comment-dwim-into-buffer content set-region #'jtsx-jsx-mode) result)) (should (equal (comment-dwim-into-buffer content set-region #'jtsx-tsx-mode) result)))) +(ert-deftest jtsx-test-uncomment-jsx-boolean-attribute-region () + (let ((set-region #'(lambda () (find-and-set-region "\\/\\* disabled \\*\\/"))) + (content "(Hello);") + (result "(Hello);")) + (should (equal (comment-dwim-into-buffer content set-region #'jtsx-jsx-mode) result)) + (should (equal (comment-dwim-into-buffer content set-region #'jtsx-tsx-mode) result)))) + (ert-deftest jtsx-test-comment-jsx-attribute-region-in-self-closing-tag () (let ((set-region #'(lambda () (find-and-set-region "disabled={false}"))) (content "();") @@ -244,6 +272,13 @@ Turn this buffer in MODE mode if supplied or defaults to jtsx-tsx-mode." (should (equal (comment-dwim-into-buffer content set-region #'jtsx-jsx-mode) result)) (should (equal (comment-dwim-into-buffer content set-region #'jtsx-tsx-mode) result)))) +(ert-deftest jtsx-test-uncomment-jsx-attribute-region-in-self-closing-tag () + (let ((set-region #'(lambda () (find-and-set-region "\\/\\* disabled={false} \\*\\/"))) + (content "();") + (result "();")) + (should (equal (comment-dwim-into-buffer content set-region #'jtsx-jsx-mode) result)) + (should (equal (comment-dwim-into-buffer content set-region #'jtsx-tsx-mode) result)))) + (ert-deftest jtsx-test-comment-jsx-boolean-attribute-region-in-self-closing-tag () (let ((set-region #'(lambda () (find-and-set-region "disabled"))) (content "();") @@ -251,6 +286,13 @@ Turn this buffer in MODE mode if supplied or defaults to jtsx-tsx-mode." (should (equal (comment-dwim-into-buffer content set-region #'jtsx-jsx-mode) result)) (should (equal (comment-dwim-into-buffer content set-region #'jtsx-tsx-mode) result)))) +(ert-deftest jtsx-test-uncomment-jsx-boolean-attribute-region-in-self-closing-tag () + (let ((set-region #'(lambda () (find-and-set-region "\\/\\* disabled \\*\\/"))) + (content "();") + (result "();")) + (should (equal (comment-dwim-into-buffer content set-region #'jtsx-jsx-mode) result)) + (should (equal (comment-dwim-into-buffer content set-region #'jtsx-tsx-mode) result)))) + (ert-deftest jtsx-test-comment-jsx-region-nested-in-attribute () (let ((set-region #'(lambda () (find-and-set-region ""))) (content "(} />);") @@ -258,6 +300,13 @@ Turn this buffer in MODE mode if supplied or defaults to jtsx-tsx-mode." (should (equal (comment-dwim-into-buffer content set-region #'jtsx-jsx-mode) result)) (should (equal (comment-dwim-into-buffer content set-region #'jtsx-tsx-mode) result)))) +(ert-deftest jtsx-test-uncomment-jsx-region-nested-in-attribute () + (let ((set-region #'(lambda () (find-and-set-region "{\\/\\* \\*\\/}"))) + (content "( */}} />);") + (result "(} />);")) + (should (equal (comment-dwim-into-buffer content set-region #'jtsx-jsx-mode) result)) + (should (equal (comment-dwim-into-buffer content set-region #'jtsx-tsx-mode) result)))) + (ert-deftest jtsx-test-comment-jsx-expression-region () (let ((set-region #'(lambda () (find-and-set-region "{'test'}"))) (content "({'test'});") @@ -265,6 +314,13 @@ Turn this buffer in MODE mode if supplied or defaults to jtsx-tsx-mode." (should (equal (comment-dwim-into-buffer content set-region #'jtsx-jsx-mode) result)) (should (equal (comment-dwim-into-buffer content set-region #'jtsx-tsx-mode) result)))) +(ert-deftest jtsx-test-uncomment-jsx-expression-region () + (let ((set-region #'(lambda () (find-and-set-region "{\\/\\* {'test'} \\*\\/}"))) + (content "({/* {'test'} */});") + (result "({'test'});")) + (should (equal (comment-dwim-into-buffer content set-region #'jtsx-jsx-mode) result)) + (should (equal (comment-dwim-into-buffer content set-region #'jtsx-tsx-mode) result)))) + (ert-deftest jtsx-test-comment-jsx-opening-element-region () (let ((set-region #'(lambda () (find-and-set-region "A"))) (content "();") @@ -272,6 +328,14 @@ Turn this buffer in MODE mode if supplied or defaults to jtsx-tsx-mode." (should (equal (comment-dwim-into-buffer content set-region #'jtsx-jsx-mode) result)) (should (equal (comment-dwim-into-buffer content set-region #'jtsx-tsx-mode) result)))) +(ert-deftest jtsx-test-uncomment-jsx-opening-element-region () + :expected-result :failed + (let ((set-region #'(lambda () (find-and-set-region "{\\/\\* A \\*\\/}"))) + (content "(<{/* A */}>);") + (result "();")) + (should (equal (comment-dwim-into-buffer content set-region #'jtsx-jsx-mode) result)) + (should (equal (comment-dwim-into-buffer content set-region #'jtsx-tsx-mode) result)))) + (ert-deftest jtsx-test-comment-jsx-closing-element-region () (let ((set-region #'(lambda () (find-and-set-region "A" 2))) (content "();") @@ -279,6 +343,41 @@ Turn this buffer in MODE mode if supplied or defaults to jtsx-tsx-mode." (should (equal (comment-dwim-into-buffer content set-region #'jtsx-jsx-mode) result)) (should (equal (comment-dwim-into-buffer content set-region #'jtsx-tsx-mode) result)))) +(ert-deftest jtsx-test-uncomment-jsx-closing-element-region () + (let ((set-region #'(lambda () (find-and-set-region "{\\/\\* A \\*\\/}"))) + (content "();") + (result "();")) + (should (equal (comment-dwim-into-buffer content set-region #'jtsx-jsx-mode) result)) + (should (equal (comment-dwim-into-buffer content set-region #'jtsx-tsx-mode) result)))) + +(ert-deftest jtsx-test-comment-jsx-nested-js-region () + (let ((set-region #'(lambda () (find-and-set-region "return "))) + (content "({[].map(()=>{return })});") + (result "({[].map(()=>{// return \n})});")) + (should (equal (comment-dwim-into-buffer content set-region #'jtsx-jsx-mode) result)) + (should (equal (comment-dwim-into-buffer content set-region #'jtsx-tsx-mode) result)))) + +(ert-deftest jtsx-test-uncomment-jsx-nested-js-region () + (let ((set-region #'(lambda () (find-and-set-region "\\/\\/ return "))) + (content "({[].map(()=>{// return \n})});") + (result "({[].map(()=>{return \n})});")) + (should (equal (comment-dwim-into-buffer content set-region #'jtsx-jsx-mode) result)) + (should (equal (comment-dwim-into-buffer content set-region #'jtsx-tsx-mode) result)))) + +(ert-deftest jtsx-test-comment-jsx-nested-js-in-attribute-region () + (let ((set-region #'(lambda () (find-and-set-region "a:1"))) + (content "();") + (result "();")) + (should (equal (comment-dwim-into-buffer content set-region #'jtsx-jsx-mode) result)) + (should (equal (comment-dwim-into-buffer content set-region #'jtsx-tsx-mode) result)))) + +(ert-deftest jtsx-test-uncomment-jsx-nested-js-in-attribute-region () + (let ((set-region #'(lambda () (find-and-set-region "\\// a:1"))) + (content "();") + (result "();")) + (should (equal (comment-dwim-into-buffer content set-region #'jtsx-jsx-mode) result)) + (should (equal (comment-dwim-into-buffer content set-region #'jtsx-tsx-mode) result)))) + (ert-deftest jtsx-test-comment-js-enclosing-jsx-at-point () (let ((set-point #'(lambda () (goto-char 0))) (content "();")