embark icon indicating copy to clipboard operation
embark copied to clipboard

Potential new target type for comments?

Open hmelman opened this issue 4 years ago • 4 comments

This isn't quite right, but I wonder if there's value to a comment target? It would give a convenient keymap when point is in a comment. These are the commands I could find of value. I'm not much of an org-mode user but its comments are different and there might be value of adding support for them.

Also, I think the README could be a little more verbose about adding things to embark-target-finders as order matters.

(defun embark-target-comment-at-point ()
  "Target the comment at point."
  (let ((state (syntax-ppss)))
    (when (nth 4 state)
      (let ((beg (nth 8 state)) end )
	(save-excursion
	  (goto-char beg)
	  (forward-comment (buffer-size))
	  (setq end (point)))
	(cons 'comment (buffer-substring beg end))))))
    
(embark-define-keymap embark-comment-map
  "Keymap for Embark comment actions."
  ("RET" comment-dwim)
  ("i" comment-indent)
  ("k" comment-kill)
  ("c" checkdoc-comments)
  ("C" checkdoc-ispell-comments)
  ("u" uncomment-region)		; probably needs cmd to mark and uncomment
  ("m" er/mark-comment)			; from expand-region
  ("s" ispell-comments-and-strings))

(add-to-list 'embark-keymap-alist '(comment . embark-comment-map))
;; also add embark-target-comment-at-point to embark-target-finders before the region check```

hmelman avatar May 22 '21 18:05 hmelman

I like the idea! Do any of the commands really take the comment contents as a string? I don't think so, and if I'm right that none do, the target finder need not return that string. (For example the region target finder just returns '(region . <region>).)

oantolin avatar May 22 '21 18:05 oantolin

No the commands don't take an argument and I wasn't sure if I needed one. Finding the beginning and end of a comment is surprisingly difficult so not needing to do so would be a win. Using er/mark-comment would also be a big win but it's an external dependency (though the needed code is small and easily recreated).

hmelman avatar May 22 '21 18:05 hmelman

Given the new multi target functionality #278, I think target finders like this became a lot more viable.

minad avatar Jul 25 '21 12:07 minad

We should do this now.

oantolin avatar Aug 15 '21 23:08 oantolin