Re: [PATCH] Add mode for automatically unhiding emphasis markers in the current region

[Shankar Rao <shankar.rao@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 9320 bytes --]

Kyle, Gustavo,

Thank you both for your detailed feedback. Attached is a new version of the
patch that incorporates most of your suggestions.

>> I have not yet signed the papers assigning copyright to the FSF.  I sent
> a
> >> request for the papers to assign@gnu.org, but have not yet received a
> >> response.
>
> Are you still waiting for a response?
>
I received an assignment form from the FSF and I just now signed it and
sent it back to them.

> +  :type '(choice (const :tag "Never unhide emphasis markers" nil)
>
> Hmm, I don't see the point of having a nil value.  That already seems
> covered by toggling org-auto-emphasis-mode on and off.
>
> I also wonder whether we can get away with not having an option here at
> all.  Is one of the below values more likely to be the predominant
> preference?
>
This mimics the behavior of prettify-symbols-mode.
org-auto-emphasis-unhide-at-point and org-auto-emphasis-mode correspond to
prettify-symbols-unprettify-at-point and global-prettify-symbols-mode,
respectively. So in the same way, one could enable prettify-symbols-mode
globally with (global-prettify-symbols-mode +1) and also have (setq
prettify-symbols-unprettify-at-point nil), which would effectively still
keep it disabled.

I'm amenable to simplifying or eliminating the option entirely. In my use I
find the 'right-edge setting to be more useful that the t setting, but the
behavior for t might be more intuitive to users. If I had to pick one
option, I would choose 'right-edge.

> I can add another thought on the matter.  While prettify-symbols is a
> > mode, org-hide-emphasis-markers is an option/variable.  So I wonder if
> > the feature would fit better the current state of things in Org as
> > either another option/defcustom or by simply expanding the range of
> > possible values for org-hide-emphasis-markers (e.g. t, nil,
> > not-at-point, not-at-point-right-edge).  But this is just a thought, I'd
> > be happy to see this feature be included either way.
>
> Hmm, yeah, that's worth thinking about.  I mentioned eliminating the
> org-auto-emphasis-unhide-at-point option, but I didn't consider doing it
> by absorbing the values into org-hide-emphasis-markers and dropping the
> mode.  Given what org-auto-emphasis-mode does underneath, though, it
> does fit quite neatly into a minor mode toggle.
>
I agree that adding this functionality as additional options to
org-hide-emphasis-markers would be more intuitive and elegant for users, as
they wouldn't have to perform  (add-hook 'org-mode
#'org-auto-emphasis-mode) in order to enable this mode. I didn't implement
it this way first because I wanted to make the simplest, least disruptive
way possible, and secondly because I didn't quite know how to implement it
:-). As you can see in the patch, besides the additional functions and
variables, my code only modifies one preexisting org function, replacing
four lines with my new eight lines.

To implement this functionality in the way Gustavo suggests, we would have
to for all users add org-auto-emphasis--post-command-hook to
post-command-hook, and modify it so that it only operates when
org-hide-emphasis-markers is 'not-at-point or 'not-at-point-right-edge. I
can try to implement this if you think it would be a good idea, but I have
one concern. Even if a user is not interested in org-auto-emphasis-mode,
this implementation would add a small amount of code to post-command-hook,
so would have the effect of slightly slowing down their system.

Shankar

On Tue, Jun 23, 2020 at 2:07 AM Kyle Meyer <kyle@kyleam.com> wrote:

> Gustavo Barros writes:
>
> > Just an user here, but I'd like to leave my +1 to this proposed feature.
> > [...]
>
> Thanks.  That's very useful feedback.
>
> > I can add another thought on the matter.  While prettify-symbols is a
> > mode, org-hide-emphasis-markers is an option/variable.  So I wonder if
> > the feature would fit better the current state of things in Org as
> > either another option/defcustom or by simply expanding the range of
> > possible values for org-hide-emphasis-markers (e.g. t, nil,
> > not-at-point, not-at-point-right-edge).  But this is just a thought, I'd
> > be happy to see this feature be included either way.
>
> Hmm, yeah, that's worth thinking about.  I mentioned eliminating the
> org-auto-emphasis-unhide-at-point option, but I didn't consider doing it
> by absorbing the values into org-hide-emphasis-markers and dropping the
> mode.  Given what org-auto-emphasis-mode does underneath, though, it
> does fit quite neatly into a minor mode toggle.
>

---
 lisp/org.el | 87 ++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 83 insertions(+), 4 deletions(-)

diff --git a/lisp/org.el b/lisp/org.el
index 4d46b4173..8bab33ec0 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -3644,6 +3644,19 @@ following symbols:
   :type 'boolean
   :safe #'booleanp)

+(defcustom org-auto-emphasis-unhide-at-point nil
+  "If non-nil, unhide the emphasis markers for the text at point.
+If set to the symbol `right-edge', also unhide the emphasis
+markers if point is immediately after the emphasized text.  The
+emphasis markers will be rehidden as soon as point moves away
+from the emphasized text.  If set to nil, the emphasis markers
+always remain hidden."
+  :package-version '(Org . "9.5")
+  :type '(choice (const :tag "Never unhide emphasis markers" nil)
+                 (const :tag "Unhide when point is inside" t)
+                 (const :tag "Unhide when point is inside or at right
edge" right-edge))
+  :group 'org-appearance)
+
 (defcustom org-hide-macro-markers nil
   "Non-nil mean font-lock should hide the brackets marking macro calls."
   :group 'org-appearance
@@ -5055,12 +5068,78 @@ stacked delimiters is N.  Escaping delimiters is
not possible."
    '(font-lock-multiline t org-emphasis t))
       (when (and org-hide-emphasis-markers
  (not (org-at-comment-p)))
- (add-text-properties (match-end 4) (match-beginning 5)
-     '(invisible org-link))
- (add-text-properties (match-beginning 3) (match-end 3)
-     '(invisible org-link)))
+ (let ((s1 (match-beginning 3))
+      (e1 (match-end 3))
+      (s2 (match-end 4))
+      (e2 (match-beginning 5)))
+  (add-text-properties s2 e2 '(invisible org-link))
+  (add-text-properties s1 e1 '(invisible org-link))
+  (add-text-properties s1 e2
+       `(org-emph-start ,s1 org-emph-end ,e2))))
       (throw :exit t))))))))

+(defvar-local org-auto-emphasis--current-region-bounds nil)
+
+(defun org-auto-emphasis--get-prop-as-list (prop)
+  "Helper function to get org-auto-emphasis properties as a list.
+If `org-auto-emphasis-unhide-at-point' is set to t, then return
+the text property PROP at point in a list. If
+`org-auto-emphasis-unhide-at-point' is set to `right-edge', the
+also include the text property PROP at point-1 unless we are at
+the beginning of the buffer."
+  (remove nil
+  (list (get-text-property (point) prop)
+ (when (and (eq org-auto-emphasis-unhide-at-point 'right-edge)
+   (not (bobp)))
+  (get-text-property (1- (point)) prop)))))
+
+(defun org-auto-emphasis--post-command-hook ()
+  ;; Rehide emphasis markers for the previous region.
+  (when (and org-auto-emphasis--current-region-bounds
+     (or (< (point) (car org-auto-emphasis--current-region-bounds))
+ (> (point) (cadr org-auto-emphasis--current-region-bounds))
+ (and (not (eq org-auto-emphasis-unhide-at-point 'right-edge))
+      (= (point) (cadr org-auto-emphasis--current-region-bounds)))))
+ (apply #'font-lock-flush org-auto-emphasis--current-region-bounds)
+ (setq org-auto-emphasis--current-region-bounds nil))
+  ;; Unhide emphasis markers for the current region.
+  (let* ((s (org-auto-emphasis--get-prop-as-list 'org-emph-start))
+ (e (and s (org-auto-emphasis--get-prop-as-list 'org-emph-end)))
+ (s (and e (apply #'min s)))
+ (e (and s (apply #'max e))))
+    (when e
+      (with-silent-modifications
+ (setq org-auto-emphasis--current-region-bounds (list s e))
+ (remove-text-properties s (1+ s) '(invisible org-link))
+ (remove-text-properties (1- e) e '(invisible org-link))))))
+
+(define-minor-mode org-auto-emphasis-mode
+  "Toggle Org Auto Emphasis mode.
+This mode, when enabled, unhides emphasis markers for the text
+at point, depending on the value of
+`org-auto-emphasis-unhide-at-point'. With a prefix argument ARG,
+enable Org Auto Emphasis mode if ARG is positive, and disable it
+otherwise.  If called from Lisp, enable the mode if ARG is
+omitted or nil.
+
+To enable this in all Org files, add the following line to init.el:
+
+  (add-hook \\='org-mode #\\='org-auto-emphasis-mode)
+"
+  :init-value nil
+  (if org-auto-emphasis-mode
+      ;; Turn on
+      (progn
+        (setq-local font-lock-extra-managed-props
+                    (append font-lock-extra-managed-props
+                            '(org-emph-start org-emph-end)))
+        (when org-auto-emphasis-unhide-at-point
+          (add-hook 'post-command-hook
+                    #'org-auto-emphasis--post-command-hook nil t))
+        (font-lock-flush))
+    ;; Turn off
+    (remove-hook 'post-command-hook #'org-auto-emphasis--post-command-hook
t)))
+
 (defun org-emphasize (&optional char)
   "Insert or change an emphasis, i.e. a font like bold or italic.
 If there is an active region, change that region to a new emphasis.
-- 
2.17.1

[-- Attachment #1.2: Type: text/html, Size: 11499 bytes --]

[-- Attachment #2: 0001-Add-mode-for-automatically-unhiding-emphasis-markers.patch --]
[-- Type: application/x-patch, Size: 6102 bytes --]