unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#57813: Icon images are non-functional
@ 2022-09-14 19:40 Juri Linkov
  2022-09-15 16:30 ` Juri Linkov
  2022-10-13  7:51 ` Juri Linkov
  0 siblings, 2 replies; 31+ messages in thread
From: Juri Linkov @ 2022-09-14 19:40 UTC (permalink / raw)
  To: 57813

(info "(elisp) Icons") suggests to use:

     (define-icon outline-open button
       '((image "right.svg" "open.xpm" "open.pbm" :height line)
         (emoji "▶️")
         (symbol "▶" "➤")
         (text "open" :face icon-button))
       "Icon used for buttons for opening a section in outline buffers."
       :version "29.1"
       :help-echo "Open this section")

So I tried this with 'C-h b', but images are displayed as black boxes.

PS: "right.svg" exists in etc/images/right.svg.





^ permalink raw reply	[flat|nested] 31+ messages in thread

* bug#57813: Icon images are non-functional
  2022-09-14 19:40 bug#57813: Icon images are non-functional Juri Linkov
@ 2022-09-15 16:30 ` Juri Linkov
  2022-09-15 16:36   ` Juri Linkov
  2022-09-15 19:53   ` Juri Linkov
  2022-10-13  7:51 ` Juri Linkov
  1 sibling, 2 replies; 31+ messages in thread
From: Juri Linkov @ 2022-09-15 16:30 UTC (permalink / raw)
  To: 57813

> (info "(elisp) Icons") suggests to use:
>
>      (define-icon outline-open button
>        '((image "right.svg" "open.xpm" "open.pbm" :height line)
>          (emoji "▶️")
>          (symbol "▶" "➤")
>          (text "open" :face icon-button))
>        "Icon used for buttons for opening a section in outline buffers."
>        :version "29.1"
>        :help-echo "Open this section")
>
> So I tried this with 'C-h b', but images are displayed as black boxes.
>
> PS: "right.svg" exists in etc/images/right.svg.

Fixed now in 471414fe6b.





^ permalink raw reply	[flat|nested] 31+ messages in thread

* bug#57813: Icon images are non-functional
  2022-09-15 16:30 ` Juri Linkov
@ 2022-09-15 16:36   ` Juri Linkov
  2022-09-15 16:53     ` Eli Zaretskii
  2022-09-15 19:53   ` Juri Linkov
  1 sibling, 1 reply; 31+ messages in thread
From: Juri Linkov @ 2022-09-15 16:36 UTC (permalink / raw)
  To: 57813

[-- Attachment #1: Type: text/plain, Size: 289 bytes --]

>> So I tried this with 'C-h b', but images are displayed as black boxes.
>>
>> PS: "right.svg" exists in etc/images/right.svg.
>
> Fixed now in 471414fe6b.

S-TAB is so slow that it takes several seconds on etc/NEWS.
Here is an attempt to optimize it, so that it's almost
instantaneous:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: outline--fix-up-all-buttons.patch --]
[-- Type: text/x-diff, Size: 1520 bytes --]

diff --git a/lisp/outline.el b/lisp/outline.el
index b19e0cf811..ff6eebcd1b 100644
--- a/lisp/outline.el
+++ b/lisp/outline.el
@@ -982,8 +989,6 @@ outline-hide-subtree
   (interactive (list last-nonmenu-event))
   (when (mouse-event-p event)
     (mouse-set-point event))
-  (when (outline--use-buttons-p)
-    (outline--insert-close-button))
   (outline-flag-subtree t))
 
 (defun outline--make-button-overlay (type)
@@ -1051,9 +1056,12 @@ outline--fix-up-all-buttons
        ;; `outline--cycle-state' will fail if we're in a totally
        ;; collapsed buffer -- but in that case, we're not in a
        ;; `show-all' situation.
-       (if (eq (ignore-errors (outline--cycle-state)) 'show-all)
-           (outline--insert-open-button)
-         (outline--insert-close-button)))
+       (if (save-excursion
+             (outline-end-of-heading)
+             (seq-some (lambda (o) (eq (overlay-get o 'invisible) 'outline))
+                       (overlays-at (point))))
+           (outline--insert-close-button)
+         (outline--insert-open-button)))
      (or from (point-min)) (or to (point-max)))))
 
 (define-obsolete-function-alias 'hide-subtree #'outline-hide-subtree "25.1")
@@ -1076,8 +1084,6 @@ outline-show-subtree
   (interactive (list last-nonmenu-event))
   (when (mouse-event-p event)
     (mouse-set-point event))
-  (when (outline--use-buttons-p)
-    (outline--insert-open-button))
   (outline-flag-subtree nil))
 
 (define-obsolete-function-alias 'show-subtree #'outline-show-subtree "25.1")

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* bug#57813: Icon images are non-functional
  2022-09-15 16:36   ` Juri Linkov
@ 2022-09-15 16:53     ` Eli Zaretskii
  2022-09-15 17:35       ` Juri Linkov
  0 siblings, 1 reply; 31+ messages in thread
From: Eli Zaretskii @ 2022-09-15 16:53 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 57813

> From: Juri Linkov <juri@linkov.net>
> Date: Thu, 15 Sep 2022 19:36:52 +0300
> 
> S-TAB is so slow that it takes several seconds on etc/NEWS.

Please profile it and post the profile.  For me, S-TAB is
instantaneous in NEWS, even though my Emacs is built without
optimizations.  So I wonder what takes it several seconds in your
case.





^ permalink raw reply	[flat|nested] 31+ messages in thread

* bug#57813: Icon images are non-functional
  2022-09-15 16:53     ` Eli Zaretskii
@ 2022-09-15 17:35       ` Juri Linkov
  2022-09-15 18:32         ` Eli Zaretskii
  0 siblings, 1 reply; 31+ messages in thread
From: Juri Linkov @ 2022-09-15 17:35 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 57813

>> S-TAB is so slow that it takes several seconds on etc/NEWS.
>
> Please profile it and post the profile.  For me, S-TAB is
> instantaneous in NEWS, even though my Emacs is built without
> optimizations.  So I wonder what takes it several seconds in your
> case.

Maybe you didn't enable buttons?  With something like:

  (setq outline-minor-mode-use-buttons '(derived-mode . special-mode))

This is before optimization:

        1995  89% - command-execute
        1959  87%  - funcall-interactively
        1946  86%   - outline-cycle-buffer
        1379  61%    - outline-hide-sublevels
        1375  61%     - outline-flag-region
        1375  61%      - outline--fix-up-all-buttons
        1371  61%       - outline-map-region
        1371  61%        - #<subr F616e6f6e796d6f75732d6c616d626461_anonymous_lambda_27>
        1339  59%         - outline--cycle-state
          43   1%          + outline-end-of-subtree
          20   0%          + seq-filter
           4   0%            outline-back-to-heading
          20   0%         + outline--insert-close-button
           4   0%     + outline-map-region
         567  25%    - outline--fix-up-all-buttons
         567  25%     - outline-map-region
         563  25%      - #<subr F616e6f6e796d6f75732d6c616d626461_anonymous_lambda_27>
         511  22%       - outline--cycle-state
         260  11%        + outline-end-of-subtree
          20   0%        + seq-filter
           4   0%          outline-back-to-heading
          16   0%       + outline--insert-close-button

This is after optimization that removed 'outline--cycle-state':

         287  82% - command-execute
         229  65%  - funcall-interactively
         216  61%   - outline-cycle-buffer
         108  30%    - outline-hide-sublevels
         104  29%     - outline-flag-region
         104  29%      - outline--fix-up-all-buttons
         104  29%       - outline-map-region
          92  26%        - #<subr F616e6f6e796d6f75732d6c616d626461_anonymous_lambda_28>
          80  22%         - outline--insert-close-button
          64  18%          - outline--make-button-overlay
          56  16%           + icon-elements
           8   2%           + seq-find
          12   3%          + define-keymap
           4   1%     + outline-map-region
         108  30%    - outline--fix-up-all-buttons
         108  30%     - outline-map-region
         104  29%      - #<subr F616e6f6e796d6f75732d6c616d626461_anonymous_lambda_28>
          88  25%       - outline--insert-close-button
          60  17%        - outline--make-button-overlay
          60  17%         + icon-elements
          20   5%        + define-keymap





^ permalink raw reply	[flat|nested] 31+ messages in thread

* bug#57813: Icon images are non-functional
  2022-09-15 17:35       ` Juri Linkov
@ 2022-09-15 18:32         ` Eli Zaretskii
  2022-09-15 19:46           ` Juri Linkov
  0 siblings, 1 reply; 31+ messages in thread
From: Eli Zaretskii @ 2022-09-15 18:32 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 57813

> From: Juri Linkov <juri@linkov.net>
> Cc: 57813@debbugs.gnu.org
> Date: Thu, 15 Sep 2022 20:35:38 +0300
> 
> >> S-TAB is so slow that it takes several seconds on etc/NEWS.
> >
> > Please profile it and post the profile.  For me, S-TAB is
> > instantaneous in NEWS, even though my Emacs is built without
> > optimizations.  So I wonder what takes it several seconds in your
> > case.
> 
> Maybe you didn't enable buttons?

Maybe you didn't tell I should?

>   With something like:
> 
>   (setq outline-minor-mode-use-buttons '(derived-mode . special-mode))

I still cannot reproduce this, after evaluating the above and visiting
NEWS.  Neither do I see buttons in NEWS.  What else is needed?  Can
you show a recipe starting from "emacs -Q"?

> This is before optimization:
> 
>         1995  89% - command-execute
>         1959  87%  - funcall-interactively
>         1946  86%   - outline-cycle-buffer
>         1379  61%    - outline-hide-sublevels
>         1375  61%     - outline-flag-region
>         1375  61%      - outline--fix-up-all-buttons
>         1371  61%       - outline-map-region
>         1371  61%        - #<subr F616e6f6e796d6f75732d6c616d626461_anonymous_lambda_27>
>         1339  59%         - outline--cycle-state
>           43   1%          + outline-end-of-subtree

And if you load outline.el (not .elc or .eln)?  How does the profile
before optimization look then?





^ permalink raw reply	[flat|nested] 31+ messages in thread

* bug#57813: Icon images are non-functional
  2022-09-15 18:32         ` Eli Zaretskii
@ 2022-09-15 19:46           ` Juri Linkov
  2022-09-16 16:16             ` Juri Linkov
  0 siblings, 1 reply; 31+ messages in thread
From: Juri Linkov @ 2022-09-15 19:46 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 57813

>>   With something like:
>>
>>   (setq outline-minor-mode-use-buttons '(derived-mode . special-mode))
>
> I still cannot reproduce this, after evaluating the above and visiting
> NEWS.  Neither do I see buttons in NEWS.  What else is needed?  Can
> you show a recipe starting from "emacs -Q"?

1. Eval: (setq outline-minor-mode-use-buttons '(derived-mode . special-mode))
2. Type 'C-h n'.
3. 'S-TAB' on a heading.

> And if you load outline.el (not .elc or .eln)?  How does the profile
> before optimization look then?

        5236  87% - command-execute
        5178  86%  - funcall-interactively
        5166  86%   - outline-cycle-buffer
        5166  86%    - let
        5166  86%     - outline--fix-up-all-buttons
        5166  86%      - if
        5166  86%       - progn
        5166  86%        - outline-map-region
        5166  86%         - save-excursion
        5166  86%          - if
        5166  86%           - progn
        5162  86%            - while
        5150  86%             - funcall
        5150  86%              - #<lambda 0x1c5f63c054af8dba>
        5150  86%               - if
        5142  86%                - eq
        5142  86%                 - condition-case
        5142  86%                  - progn
        5142  86%                   - outline--cycle-state
        5138  86%                    - save-excursion
        5138  86%                     - let
        3718  62%                      + outline-back-to-heading
         824  13%                      + outline-end-of-subtree
         314   5%                      + cond
         274   4%                      + setq
           8   0%                      + outline-end-of-heading

The patch removes the problematic outline-back-to-heading and
outline-end-of-subtree.





^ permalink raw reply	[flat|nested] 31+ messages in thread

* bug#57813: Icon images are non-functional
  2022-09-15 16:30 ` Juri Linkov
  2022-09-15 16:36   ` Juri Linkov
@ 2022-09-15 19:53   ` Juri Linkov
  2022-09-16  5:29     ` Eli Zaretskii
  2022-09-23 15:48     ` Juri Linkov
  1 sibling, 2 replies; 31+ messages in thread
From: Juri Linkov @ 2022-09-15 19:53 UTC (permalink / raw)
  To: 57813

[-- Attachment #1: Type: text/plain, Size: 147 bytes --]

> Fixed now in 471414fe6b.

Since now images can be displayed in outlines,
here is a patch that adds nice images to outlines
that look like this:


[-- Attachment #2: outlines.png --]
[-- Type: image/png, Size: 73601 bytes --]

[-- Attachment #3: outline-svg.patch --]
[-- Type: text/x-diff, Size: 2185 bytes --]

diff --git a/etc/images/outline-close.svg b/etc/images/outline-close.svg
new file mode 100644
index 0000000000..ea9157a5fb
--- /dev/null
+++ b/etc/images/outline-close.svg
@@ -0,0 +1,6 @@
+<svg xmlns="http://www.w3.org/2000/svg" width="20" height="20" viewBox="0 0 20 20">
+<title>outline-close</title>
+<g transform="rotate(-90, 10, 10)">
+<path d="m17.5 4.75-7.5 7.5-7.5-7.5L1 6.25l9 9 9-9z"/>
+</g>
+</svg>
diff --git a/etc/images/outline-open.svg b/etc/images/outline-open.svg
new file mode 100644
index 0000000000..75cf6aff9f
--- /dev/null
+++ b/etc/images/outline-open.svg
@@ -0,0 +1,4 @@
+<svg xmlns="http://www.w3.org/2000/svg" width="20" height="20" viewBox="0 0 20 20">
+<title>outline-open</title>
+<path d="m17.5 4.75-7.5 7.5-7.5-7.5L1 6.25l9 9 9-9z"/>
+</svg>
diff --git a/lisp/emacs-lisp/icons.el b/lisp/emacs-lisp/icons.el
index 93749a3451..ff4f20c207 100644
--- a/lisp/emacs-lisp/icons.el
+++ b/lisp/emacs-lisp/icons.el
@@ -202,7 +202,7 @@ icons--create
                             :height (if (eq height 'line)
                                         (window-default-line-height)
                                       height)
-                            :scale 1)
+                            :scale 1 :ascent 'center)
             (create-image file))))))
 
 (cl-defmethod icons--create ((_type (eql 'emoji)) icon _keywords)
diff --git a/lisp/outline.el b/lisp/outline.el
index b19e0cf811..364c3bee84 100644
--- a/lisp/outline.el
+++ b/lisp/outline.el
@@ -292,16 +295,18 @@ outline-minor-mode-use-buttons
   :safe #'booleanp
   :version "29.1")
 
-(define-icon outline-open button
-  '((emoji "🔽")
+(define-icon outline-open nil
+  '((image "outline-open.svg" :height 15 :ascent center)
+    (emoji "🔽")
     (symbol " ▼ ")
     (text " open "))
   "Icon used for buttons for opening a section in outline buffers."
   :version "29.1"
   :help-echo "Open this section")
 
-(define-icon outline-close button
-  '((emoji "▶️")
+(define-icon outline-close nil
+  '((image "outline-close.svg" :height 15 :ascent center)
+    (emoji "▶️")
     (symbol " ▶ ")
     (text " close "))
   "Icon used for buttons for closing a section in outline buffers."

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* bug#57813: Icon images are non-functional
  2022-09-15 19:53   ` Juri Linkov
@ 2022-09-16  5:29     ` Eli Zaretskii
  2022-09-16  7:09       ` Juri Linkov
  2022-09-23 15:48     ` Juri Linkov
  1 sibling, 1 reply; 31+ messages in thread
From: Eli Zaretskii @ 2022-09-16  5:29 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 57813

> From: Juri Linkov <juri@linkov.net>
> Date: Thu, 15 Sep 2022 22:53:07 +0300
> 
> Since now images can be displayed in outlines,
> here is a patch that adds nice images to outlines
> that look like this:

Thanks, LGTM.





^ permalink raw reply	[flat|nested] 31+ messages in thread

* bug#57813: Icon images are non-functional
  2022-09-16  5:29     ` Eli Zaretskii
@ 2022-09-16  7:09       ` Juri Linkov
  2022-09-17 19:37         ` Juri Linkov
  0 siblings, 1 reply; 31+ messages in thread
From: Juri Linkov @ 2022-09-16  7:09 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 57813

>> Since now images can be displayed in outlines,
>> here is a patch that adds nice images to outlines
>> that look like this:
>
> Thanks, LGTM.

So now pushed to master.  Next I could try to add an option
to display these outline images in margins.





^ permalink raw reply	[flat|nested] 31+ messages in thread

* bug#57813: Icon images are non-functional
  2022-09-15 19:46           ` Juri Linkov
@ 2022-09-16 16:16             ` Juri Linkov
  0 siblings, 0 replies; 31+ messages in thread
From: Juri Linkov @ 2022-09-16 16:16 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 57813

> The patch removes the problematic outline-back-to-heading and
> outline-end-of-subtree.

This is pushed as well.





^ permalink raw reply	[flat|nested] 31+ messages in thread

* bug#57813: Icon images are non-functional
  2022-09-16  7:09       ` Juri Linkov
@ 2022-09-17 19:37         ` Juri Linkov
  2022-09-18  5:08           ` Eli Zaretskii
  2022-09-18  5:17           ` Eli Zaretskii
  0 siblings, 2 replies; 31+ messages in thread
From: Juri Linkov @ 2022-09-17 19:37 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 57813

[-- Attachment #1: Type: text/plain, Size: 249 bytes --]

> Next I could try to add an option to display these outline images in
> margins.

Here is a preliminary patch, but using buffer-local left-margin-width
exhibits strange behavior: margins don't always appear immediately
after switching the buffer.


[-- Attachment #2: outline-minor-mode-use-margins.png --]
[-- Type: image/png, Size: 32879 bytes --]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: outline-minor-mode-use-margins.patch --]
[-- Type: text/x-diff, Size: 6757 bytes --]

diff --git a/lisp/outline.el b/lisp/outline.el
index 25ef1616b9..3b8e68bfe1 100644
--- a/lisp/outline.el
+++ b/lisp/outline.el
@@ -292,6 +292,16 @@ outline-minor-mode-use-buttons
   :safe #'booleanp
   :version "29.1")
 
+(defcustom outline-minor-mode-use-margins '(derived-mode . special-mode)
+  "Whether to display clickable buttons on the margins.
+The value should be a `buffer-match-p' condition.
+
+These buttons can be used to hide and show the body under the heading.
+Note that this feature is meant to be used in editing buffers."
+  :type 'buffer-predicate
+  :safe #'booleanp
+  :version "29.1")
+
 (define-icon outline-open nil
   '((image "outline-open.svg" "outline-open.pbm"
            :height 15 :ascent center)
@@ -437,8 +447,10 @@ outline-minor-mode-highlight-buffer
                     (and (eq outline-minor-mode-highlight t)
                          (not (get-text-property (match-beginning 0) 'face))))
             (overlay-put overlay 'face (outline-font-lock-face)))
-          (when (outline--use-buttons-p)
-            (outline--insert-open-button)))
+          (cond ((outline--use-margins-p)
+                 (outline--insert-open-button t))
+                ((outline--use-buttons-p)
+                 (outline--insert-open-button))))
         (goto-char (match-end 0))))))
 
 ;;;###autoload
@@ -453,6 +465,9 @@ outline-minor-mode
             (key-description outline-minor-mode-prefix) outline-mode-prefix-map)
   (if outline-minor-mode
       (progn
+        (when (outline--use-margins-p)
+          (setq-local left-margin-width (1+ left-margin-width))
+          (setq-local fringes-outside-margins t))
         (when outline-minor-mode-highlight
           (if (and global-font-lock-mode (font-lock-specified-p major-mode))
               (progn
@@ -473,6 +488,9 @@ outline-minor-mode
           (font-lock-remove-keywords nil outline-font-lock-keywords))
       (remove-overlays nil nil 'outline-overlay t)
       (font-lock-flush))
+    (when (outline--use-margins-p)
+      (setq-local left-margin-width (1- left-margin-width))
+      (setq-local fringes-outside-margins nil))
     (setq line-move-ignore-invisible nil)
     ;; Cause use of ellipses for invisible text.
     (remove-from-invisibility-spec '(outline . t))
@@ -483,6 +501,10 @@ outline--use-buttons-p
   (and outline-minor-mode
        (buffer-match-p outline-minor-mode-use-buttons (current-buffer))))
 
+(defun outline--use-margins-p ()
+  (and outline-minor-mode
+       (buffer-match-p outline-minor-mode-use-margins (current-buffer))))
+
 (defvar-local outline-heading-alist ()
   "Alist associating a heading for every possible level.
 Each entry is of the form (HEADING . LEVEL).
@@ -1012,10 +1034,34 @@ outline--make-button-overlay
         (overlay-put o 'face (plist-get icon 'face))))
     o))
 
-(defun outline--insert-open-button ()
+(defun outline--make-margin-overlay (type)
+  (let ((o (seq-find (lambda (o)
+                       (overlay-get o 'outline-margin))
+                     (overlays-at (point)))))
+    (unless o
+      (setq o (make-overlay (point) (1+ (point))))
+      (overlay-put o 'follow-link 'mouse-face)
+      (overlay-put o 'mouse-face 'highlight)
+      (overlay-put o 'outline-margin t))
+    (let ((icon
+           (icon-elements (if (eq type 'close) 'outline-close 'outline-open)))
+          (inhibit-read-only t))
+      (overlay-put o 'before-string
+                   (propertize " " 'display `((margin left-margin)
+                                              ,(or (plist-get icon 'image)
+                                                   (plist-get icon 'string))))))
+    o))
+
+(defun outline--insert-open-button (&optional margins-p)
   (with-silent-modifications
     (save-excursion
-        (beginning-of-line)
+      (beginning-of-line)
+      (if margins-p
+          (let ((o (outline--make-margin-overlay 'open)))
+            (overlay-put o 'help-echo "Click to hide")
+            (overlay-put o 'keymap
+                         (define-keymap
+                           "<mouse-2>" #'outline-hide-subtree)))
         (when (derived-mode-p 'special-mode)
           (let ((inhibit-read-only t))
             (insert "  ")
@@ -1025,12 +1071,18 @@ outline--insert-open-button
           (overlay-put o 'keymap
                        (define-keymap
                          "RET" #'outline-hide-subtree
-                         "<mouse-2>" #'outline-hide-subtree))))))
+                         "<mouse-2>" #'outline-hide-subtree)))))))
 
-(defun outline--insert-close-button ()
+(defun outline--insert-close-button (&optional margins-p)
   (with-silent-modifications
     (save-excursion
-        (beginning-of-line)
+      (beginning-of-line)
+      (if margins-p
+          (let ((o (outline--make-margin-overlay 'close)))
+            (overlay-put o 'help-echo "Click to show")
+            (overlay-put o 'keymap
+                         (define-keymap
+                           "<mouse-2>" #'outline-show-subtree)))
         (when (derived-mode-p 'special-mode)
           (let ((inhibit-read-only t))
             (insert "  ")
@@ -1040,23 +1092,25 @@ outline--insert-close-button
           (overlay-put o 'keymap
                        (define-keymap
                          "RET" #'outline-show-subtree
-                         "<mouse-2>" #'outline-show-subtree))))))
+                         "<mouse-2>" #'outline-show-subtree)))))))
 
 (defun outline--fix-up-all-buttons (&optional from to)
-  (when (outline--use-buttons-p)
-    (when from
-      (save-excursion
-        (goto-char from)
-        (setq from (line-beginning-position))))
-    (outline-map-region
-     (lambda ()
-       (if (save-excursion
-             (outline-end-of-heading)
-             (seq-some (lambda (o) (eq (overlay-get o 'invisible) 'outline))
-                       (overlays-at (point))))
-           (outline--insert-close-button)
-         (outline--insert-open-button)))
-     (or from (point-min)) (or to (point-max)))))
+  (let ((buttons-p (outline--use-buttons-p))
+        (margins-p (outline--use-margins-p)))
+    (when (or buttons-p margins-p)
+      (when from
+        (save-excursion
+          (goto-char from)
+          (setq from (line-beginning-position))))
+      (outline-map-region
+       (lambda ()
+         (if (save-excursion
+               (outline-end-of-heading)
+               (seq-some (lambda (o) (eq (overlay-get o 'invisible) 'outline))
+                         (overlays-at (point))))
+             (outline--insert-close-button margins-p)
+           (outline--insert-open-button margins-p)))
+       (or from (point-min)) (or to (point-max))))))
 
 (define-obsolete-function-alias 'hide-subtree #'outline-hide-subtree "25.1")
 

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* bug#57813: Icon images are non-functional
  2022-09-17 19:37         ` Juri Linkov
@ 2022-09-18  5:08           ` Eli Zaretskii
  2022-09-18 18:46             ` Juri Linkov
  2022-09-18  5:17           ` Eli Zaretskii
  1 sibling, 1 reply; 31+ messages in thread
From: Eli Zaretskii @ 2022-09-18  5:08 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 57813

> From: Juri Linkov <juri@linkov.net>
> Cc: 57813@debbugs.gnu.org
> Date: Sat, 17 Sep 2022 22:37:22 +0300
> 
> Here is a preliminary patch, but using buffer-local left-margin-width
> exhibits strange behavior: margins don't always appear immediately
> after switching the buffer.

Margins are specific to a window, not to a buffer.  See the
description of set-window-margins in the ELisp manual, and the
accompanying description of set-window-buffer there.






^ permalink raw reply	[flat|nested] 31+ messages in thread

* bug#57813: Icon images are non-functional
  2022-09-17 19:37         ` Juri Linkov
  2022-09-18  5:08           ` Eli Zaretskii
@ 2022-09-18  5:17           ` Eli Zaretskii
  2022-09-18 19:06             ` Juri Linkov
  1 sibling, 1 reply; 31+ messages in thread
From: Eli Zaretskii @ 2022-09-18  5:17 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 57813

> From: Juri Linkov <juri@linkov.net>
> Cc: 57813@debbugs.gnu.org
> Date: Sat, 17 Sep 2022 22:37:22 +0300
> 
> Here is a preliminary patch

Thanks.  I only quickly read the patch, and have one comment: the
hard-coded left-margin should depend on the value returned by
current-bidi-paragraph-direction, because text that is displayed
right-to-left should have the icons on the right margin, not left
margin.





^ permalink raw reply	[flat|nested] 31+ messages in thread

* bug#57813: Icon images are non-functional
  2022-09-18  5:08           ` Eli Zaretskii
@ 2022-09-18 18:46             ` Juri Linkov
  0 siblings, 0 replies; 31+ messages in thread
From: Juri Linkov @ 2022-09-18 18:46 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 57813

>> Here is a preliminary patch, but using buffer-local left-margin-width
>> exhibits strange behavior: margins don't always appear immediately
>> after switching the buffer.
>
> Margins are specific to a window, not to a buffer.  See the
> description of set-window-margins in the ELisp manual, and the
> accompanying description of set-window-buffer there.

 -- Function: set-window-buffer window buffer-or-name &optional
          keep-margins
     ...
     By default, this function resets WINDOW’s position, display
     margins, fringe widths, and scroll bar settings, based on the local
     variables in the specified buffer.  However, if the optional
     argument KEEP-MARGINS is non-‘nil’, it leaves WINDOW’s display
     margins, fringes and scroll bar settings alone.

KEEP-MARGINS is nil by default, so buffer-local 'left-margin-width'
should work as described:

 -- Variable: left-margin-width
     This variable specifies the width of the left margin, in character
     cell (a.k.a. “column”) units.  It is buffer-local in all buffers.
     A value of ‘nil’ means no left marginal area.

And indeed, 'left-margin-width' works fine.  The only problem is
with 'C-h n' that doesn't show margins immediately, only after
switching to another buffer and back.  Then afterwards buffer-local
margins are kept in etc/NEWS as well.  It can be fixed by:

diff --git a/lisp/textmodes/emacs-news-mode.el b/lisp/textmodes/emacs-news-mode.el
index 88e8948060..9b99740290 100644
--- a/lisp/textmodes/emacs-news-mode.el
+++ b/lisp/textmodes/emacs-news-mode.el
@@ -80,6 +80,7 @@ emacs-news--mode-common
               outline-minor-mode-cycle t
               outline-minor-mode-highlight 'append)
   (outline-minor-mode)
+  (set-window-buffer nil (current-buffer))
   (emacs-etc--hide-local-variables))





^ permalink raw reply related	[flat|nested] 31+ messages in thread

* bug#57813: Icon images are non-functional
  2022-09-18  5:17           ` Eli Zaretskii
@ 2022-09-18 19:06             ` Juri Linkov
  2022-09-19 12:12               ` Eli Zaretskii
  0 siblings, 1 reply; 31+ messages in thread
From: Juri Linkov @ 2022-09-18 19:06 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 57813

[-- Attachment #1: Type: text/plain, Size: 793 bytes --]

>> Here is a preliminary patch
>
> Thanks.  I only quickly read the patch, and have one comment: the
> hard-coded left-margin should depend on the value returned by
> current-bidi-paragraph-direction, because text that is displayed
> right-to-left should have the icons on the right margin, not left
> margin.

To not create a new mirrored image file for RTL arrow required a change in
icons--create to support ':rotation 180' for the existing image file:

  (define-icon outline-close-rtl-in-margins outline-close
    '((image "outline-close.svg" "outline-close.pbm"
             :height 10 :ascent center :rotation 180))
    "Icon used for buttons for closing a section in outline buffers."
    :version "29.1"
    :help-echo "Close this section")

Then the right margin could be used with:

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: outline-minor-mode-rtl-in-margins.patch --]
[-- Type: text/x-diff, Size: 8603 bytes --]

diff --git a/lisp/emacs-lisp/icons.el b/lisp/emacs-lisp/icons.el
index ff4f20c207..96f5ea6b08 100644
--- a/lisp/emacs-lisp/icons.el
+++ b/lisp/emacs-lisp/icons.el
@@ -202,7 +202,9 @@ icons--create
                             :height (if (eq height 'line)
                                         (window-default-line-height)
                                       height)
-                            :scale 1 :ascent 'center)
+                            :scale 1
+                            :rotation (plist-get keywords :rotation)
+                            :ascent (or (plist-get keywords :ascent) 'center))
             (create-image file))))))
 
 (cl-defmethod icons--create ((_type (eql 'emoji)) icon _keywords)
diff --git a/lisp/outline.el b/lisp/outline.el
index 25ef1616b9..32c5799814 100644
--- a/lisp/outline.el
+++ b/lisp/outline.el
@@ -292,6 +292,16 @@ outline-minor-mode-use-buttons
   :safe #'booleanp
   :version "29.1")
 
+(defcustom outline-minor-mode-use-margins '(derived-mode . special-mode)
+  "Whether to display clickable buttons on the margins.
+The value should be a `buffer-match-p' condition.
+
+These buttons can be used to hide and show the body under the heading.
+Note that this feature is meant to be used in editing buffers."
+  :type 'buffer-predicate
+  :safe #'booleanp
+  :version "29.1")
+
 (define-icon outline-open nil
   '((image "outline-open.svg" "outline-open.pbm"
            :height 15 :ascent center)
@@ -312,6 +322,27 @@ outline-close
   :version "29.1"
   :help-echo "Close this section")
 
+(define-icon outline-open-in-margins outline-open
+  '((image "outline-open.svg" "outline-open.pbm"
+           :height 10 :ascent center))
+  "Icon used for buttons for opening a section in outline buffers."
+  :version "29.1"
+  :help-echo "Open this section")
+
+(define-icon outline-close-in-margins outline-close
+  '((image "outline-open.svg" "outline-open.pbm"
+           :height 10 :ascent center :rotation -90))
+  "Icon used for buttons for closing a section in outline buffers."
+  :version "29.1"
+  :help-echo "Close this section")
+
+(define-icon outline-close-rtl-in-margins outline-close
+  '((image "outline-open.svg" "outline-open.pbm"
+           :height 10 :ascent center :rotation 90))
+  "Icon used for buttons for closing a section in outline buffers."
+  :version "29.1"
+  :help-echo "Close this section")
+
 \f
 (defvar outline-level #'outline-level
   "Function of no args to compute a header's nesting level in an outline.
@@ -453,6 +486,11 @@ outline-minor-mode
             (key-description outline-minor-mode-prefix) outline-mode-prefix-map)
   (if outline-minor-mode
       (progn
+        (when (outline--use-margins-p)
+          (if (eq (current-bidi-paragraph-direction) 'right-to-left)
+              (setq-local right-margin-width (1+ right-margin-width))
+            (setq-local left-margin-width (1+ left-margin-width)))
+          (setq-local fringes-outside-margins t))
         (when outline-minor-mode-highlight
           (if (and global-font-lock-mode (font-lock-specified-p major-mode))
               (progn
@@ -473,6 +511,11 @@ outline-minor-mode
           (font-lock-remove-keywords nil outline-font-lock-keywords))
       (remove-overlays nil nil 'outline-overlay t)
       (font-lock-flush))
+    (when (outline--use-margins-p)
+      (if (eq (current-bidi-paragraph-direction) 'right-to-left)
+          (setq-local right-margin-width (1- right-margin-width))
+        (setq-local left-margin-width (1- left-margin-width)))
+      (setq-local fringes-outside-margins nil))
     (setq line-move-ignore-invisible nil)
     ;; Cause use of ellipses for invisible text.
     (remove-from-invisibility-spec '(outline . t))
@@ -483,6 +526,10 @@ outline--use-buttons-p
   (and outline-minor-mode
        (buffer-match-p outline-minor-mode-use-buttons (current-buffer))))
 
+(defun outline--use-margins-p ()
+  (and outline-minor-mode
+       (buffer-match-p outline-minor-mode-use-margins (current-buffer))))
+
 (defvar-local outline-heading-alist ()
   "Alist associating a heading for every possible level.
 Each entry is of the form (HEADING . LEVEL).
@@ -1012,10 +1061,43 @@ outline--make-button-overlay
         (overlay-put o 'face (plist-get icon 'face))))
     o))
 
-(defun outline--insert-open-button ()
+(defun outline--make-margin-overlay (type)
+  (let ((o (seq-find (lambda (o)
+                       (overlay-get o 'outline-margin))
+                     (overlays-at (point)))))
+    (unless o
+      (setq o (make-overlay (point) (1+ (point))))
+      (overlay-put o 'follow-link 'mouse-face)
+      (overlay-put o 'mouse-face 'highlight)
+      (overlay-put o 'outline-margin t))
+    (let ((icon
+           (icon-elements (if (eq type 'close)
+                              (if (eq (current-bidi-paragraph-direction)
+                                      'right-to-left)
+                                  'outline-close-rtl-in-margins
+                                'outline-close-in-margins)
+                            'outline-open-in-margins)))
+          (inhibit-read-only t))
+      (overlay-put
+       o 'before-string
+       (propertize " " 'display
+                   `((margin ,(if (eq (current-bidi-paragraph-direction)
+                                      'right-to-left)
+                                  'right-margin 'left-margin))
+                     ,(or (plist-get icon 'image)
+                          (plist-get icon 'string))))))
+    o))
+
+(defun outline--insert-open-button (&optional margins-p)
   (with-silent-modifications
     (save-excursion
-        (beginning-of-line)
+      (beginning-of-line)
+      (if margins-p
+          (let ((o (outline--make-margin-overlay 'open)))
+            (overlay-put o 'help-echo "Click to hide")
+            (overlay-put o 'keymap
+                         (define-keymap
+                           "<mouse-2>" #'outline-hide-subtree)))
         (when (derived-mode-p 'special-mode)
           (let ((inhibit-read-only t))
             (insert "  ")
@@ -1025,12 +1107,18 @@ outline--insert-open-button
           (overlay-put o 'keymap
                        (define-keymap
                          "RET" #'outline-hide-subtree
-                         "<mouse-2>" #'outline-hide-subtree))))))
+                         "<mouse-2>" #'outline-hide-subtree)))))))
 
-(defun outline--insert-close-button ()
+(defun outline--insert-close-button (&optional margins-p)
   (with-silent-modifications
     (save-excursion
-        (beginning-of-line)
+      (beginning-of-line)
+      (if margins-p
+          (let ((o (outline--make-margin-overlay 'close)))
+            (overlay-put o 'help-echo "Click to show")
+            (overlay-put o 'keymap
+                         (define-keymap
+                           "<mouse-2>" #'outline-show-subtree)))
         (when (derived-mode-p 'special-mode)
           (let ((inhibit-read-only t))
             (insert "  ")
@@ -1040,23 +1128,25 @@ outline--insert-close-button
           (overlay-put o 'keymap
                        (define-keymap
                          "RET" #'outline-show-subtree
-                         "<mouse-2>" #'outline-show-subtree))))))
+                         "<mouse-2>" #'outline-show-subtree)))))))
 
 (defun outline--fix-up-all-buttons (&optional from to)
-  (when (outline--use-buttons-p)
-    (when from
-      (save-excursion
-        (goto-char from)
-        (setq from (line-beginning-position))))
-    (outline-map-region
-     (lambda ()
-       (if (save-excursion
-             (outline-end-of-heading)
-             (seq-some (lambda (o) (eq (overlay-get o 'invisible) 'outline))
-                       (overlays-at (point))))
-           (outline--insert-close-button)
-         (outline--insert-open-button)))
-     (or from (point-min)) (or to (point-max)))))
+  (let ((buttons-p (outline--use-buttons-p))
+        (margins-p (outline--use-margins-p)))
+    (when (or buttons-p margins-p)
+      (when from
+        (save-excursion
+          (goto-char from)
+          (setq from (line-beginning-position))))
+      (outline-map-region
+       (lambda ()
+         (if (save-excursion
+               (outline-end-of-heading)
+               (seq-some (lambda (o) (eq (overlay-get o 'invisible) 'outline))
+                         (overlays-at (point))))
+             (outline--insert-close-button margins-p)
+           (outline--insert-open-button margins-p)))
+       (or from (point-min)) (or to (point-max))))))
 
 (define-obsolete-function-alias 'hide-subtree #'outline-hide-subtree "25.1")
 

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* bug#57813: Icon images are non-functional
  2022-09-18 19:06             ` Juri Linkov
@ 2022-09-19 12:12               ` Eli Zaretskii
  2022-09-19 19:37                 ` Juri Linkov
  0 siblings, 1 reply; 31+ messages in thread
From: Eli Zaretskii @ 2022-09-19 12:12 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 57813

> From: Juri Linkov <juri@linkov.net>
> Cc: 57813@debbugs.gnu.org
> Date: Sun, 18 Sep 2022 22:06:56 +0300
> 
> > Thanks.  I only quickly read the patch, and have one comment: the
> > hard-coded left-margin should depend on the value returned by
> > current-bidi-paragraph-direction, because text that is displayed
> > right-to-left should have the icons on the right margin, not left
> > margin.
> 
> To not create a new mirrored image file for RTL arrow required a change in
> icons--create to support ':rotation 180' for the existing image file:
> 
>   (define-icon outline-close-rtl-in-margins outline-close
>     '((image "outline-close.svg" "outline-close.pbm"
>              :height 10 :ascent center :rotation 180))
>     "Icon used for buttons for closing a section in outline buffers."
>     :version "29.1"
>     :help-echo "Close this section")
> 
> Then the right margin could be used with:

Thanks, the code LGTM.





^ permalink raw reply	[flat|nested] 31+ messages in thread

* bug#57813: Icon images are non-functional
  2022-09-19 12:12               ` Eli Zaretskii
@ 2022-09-19 19:37                 ` Juri Linkov
  2022-09-20 11:31                   ` Eli Zaretskii
  0 siblings, 1 reply; 31+ messages in thread
From: Juri Linkov @ 2022-09-19 19:37 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 57813

>> Then the right margin could be used with:
>
> Thanks, the code LGTM.

Pushed.  There is still a remaining problem: mouse clicks in the margins
have no effect.  Then I noticed in (info "(elisp) Display Margins"):

  There is currently no way to make text or images in the margin
  mouse-sensitive.

This means no way to bind mouse events in the margins?





^ permalink raw reply	[flat|nested] 31+ messages in thread

* bug#57813: Icon images are non-functional
  2022-09-19 19:37                 ` Juri Linkov
@ 2022-09-20 11:31                   ` Eli Zaretskii
  2022-09-20 16:12                     ` Juri Linkov
  0 siblings, 1 reply; 31+ messages in thread
From: Eli Zaretskii @ 2022-09-20 11:31 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 57813

> From: Juri Linkov <juri@linkov.net>
> Cc: 57813@debbugs.gnu.org
> Date: Mon, 19 Sep 2022 22:37:08 +0300
> 
> >> Then the right margin could be used with:
> >
> > Thanks, the code LGTM.
> 
> Pushed.  There is still a remaining problem: mouse clicks in the margins
> have no effect.  Then I noticed in (info "(elisp) Display Margins"):
> 
>   There is currently no way to make text or images in the margin
>   mouse-sensitive.
> 
> This means no way to bind mouse events in the margins?

When I click the mouse in a left margin, "C-h c" says:

  <left-margin> <mouse-1> (translated from <down-mouse-1> <mouse-1>) at that spot is undefined

So it looks like this is possible, and that text in the manual is
simply outdated?





^ permalink raw reply	[flat|nested] 31+ messages in thread

* bug#57813: Icon images are non-functional
  2022-09-20 11:31                   ` Eli Zaretskii
@ 2022-09-20 16:12                     ` Juri Linkov
  0 siblings, 0 replies; 31+ messages in thread
From: Juri Linkov @ 2022-09-20 16:12 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 57813

>> Pushed.  There is still a remaining problem: mouse clicks in the margins
>> have no effect.  Then I noticed in (info "(elisp) Display Margins"):
>>
>>   There is currently no way to make text or images in the margin
>>   mouse-sensitive.
>>
>> This means no way to bind mouse events in the margins?
>
> When I click the mouse in a left margin, "C-h c" says:
>
>   <left-margin> <mouse-1> (translated from <down-mouse-1> <mouse-1>) at that spot is undefined
>
> So it looks like this is possible, and that text in the manual is
> simply outdated?

These are mode bindings for the whole buffer, but I expected margin-local
bindings that really don't work, so probably the manual is right.
Now I added mode bindings, and everything works fine.





^ permalink raw reply	[flat|nested] 31+ messages in thread

* bug#57813: Icon images are non-functional
  2022-09-15 19:53   ` Juri Linkov
  2022-09-16  5:29     ` Eli Zaretskii
@ 2022-09-23 15:48     ` Juri Linkov
  2022-09-23 16:26       ` Lars Ingebrigtsen
  1 sibling, 1 reply; 31+ messages in thread
From: Juri Linkov @ 2022-09-23 15:48 UTC (permalink / raw)
  To: 57813

> diff --git a/lisp/emacs-lisp/icons.el b/lisp/emacs-lisp/icons.el
> index ff4f20c207..ccc3657793 100644
> --- a/lisp/emacs-lisp/icons.el
> +++ b/lisp/emacs-lisp/icons.el
> @@ -202,7 +202,11 @@ icons--create
>                              :height (if (eq height 'line)
>                                          (window-default-line-height)
>                                        height)
> -                            :scale 1 :ascent 'center)
> +                            :scale 1
> +                            :rotation (plist-get keywords :rotation)

Oh, I noticed now this prints a warning in *Messages*:

  Invalid image ‘:rotation’ parameter nil

but still continues to work fine.

So maybe the keyword :rotation should be used only on this
condition (plist-member keywords :rotation) instead of:

              (create-image file
                            nil nil
                            :rotation (plist-get keywords :rotation)

I guess the only way is to convert this function call to 'apply'
with a constructed list of arguments?

Or better to change compute_image_rotation to allow nil in :rotation:

diff --git a/src/image.c b/src/image.c
index 1e323ba66a..6d9316c64b 100644
--- a/src/image.c
+++ b/src/image.c
@@ -2548,6 +2548,8 @@ compute_image_rotation (struct image *img, double *rotation)
   Lisp_Object value = image_spec_value (img->spec, QCrotation, &foundp);
   if (!foundp)
     return;
+  if (NILP (value))
+    return;
   if (! NUMBERP (value))
     {
       image_error ("Invalid image `:rotation' parameter");





^ permalink raw reply related	[flat|nested] 31+ messages in thread

* bug#57813: Icon images are non-functional
  2022-09-23 15:48     ` Juri Linkov
@ 2022-09-23 16:26       ` Lars Ingebrigtsen
  2022-09-24 17:17         ` Juri Linkov
  0 siblings, 1 reply; 31+ messages in thread
From: Lars Ingebrigtsen @ 2022-09-23 16:26 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 57813

Juri Linkov <juri@linkov.net> writes:

>               (create-image file
>                             nil nil
>                             :rotation (plist-get keywords :rotation)

You could just have a :rotation (or ... 0) here, I think?






^ permalink raw reply	[flat|nested] 31+ messages in thread

* bug#57813: Icon images are non-functional
  2022-09-23 16:26       ` Lars Ingebrigtsen
@ 2022-09-24 17:17         ` Juri Linkov
  2022-10-12 14:42           ` Max Brieiev
  0 siblings, 1 reply; 31+ messages in thread
From: Juri Linkov @ 2022-09-24 17:17 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 57813

>>               (create-image file
>>                             nil nil
>>                             :rotation (plist-get keywords :rotation)
>
> You could just have a :rotation (or ... 0) here, I think?

Ok, done.  (I checked that anyway the default value is 0.0
in image_set_transform.)





^ permalink raw reply	[flat|nested] 31+ messages in thread

* bug#57813: Icon images are non-functional
  2022-09-24 17:17         ` Juri Linkov
@ 2022-10-12 14:42           ` Max Brieiev
  2022-10-12 18:55             ` Juri Linkov
  0 siblings, 1 reply; 31+ messages in thread
From: Max Brieiev @ 2022-10-12 14:42 UTC (permalink / raw)
  To: Juri Linkov; +Cc: Lars Ingebrigtsen, 57813

[-- Attachment #1: Type: text/plain, Size: 109 bytes --]

It seems in some cases this can cause an overlap between the heading and
the image, like on the screenshot:


[-- Attachment #2: screenshot --]
[-- Type: image/png, Size: 46020 bytes --]

^ permalink raw reply	[flat|nested] 31+ messages in thread

* bug#57813: Icon images are non-functional
  2022-10-12 14:42           ` Max Brieiev
@ 2022-10-12 18:55             ` Juri Linkov
  2022-10-23 16:55               ` Juri Linkov
  0 siblings, 1 reply; 31+ messages in thread
From: Juri Linkov @ 2022-10-12 18:55 UTC (permalink / raw)
  To: Max Brieiev; +Cc: Lars Ingebrigtsen, 57813

> It seems in some cases this can cause an overlap between the heading and
> the image, like on the screenshot:

It's hard to guess whether the outlines are using buttons or margins.
Probably buttons, because margins are not displayed in *Completions*.
Actually the same problem exists for both of them.  By default the
button/margin face is inherited, because in most modes inheriting
fontification from the outline heading lines provides a nicer look,
but your example demonstrates that in some cases that face needs to be
overridden.  Then there are several variants what face to use instead:
1. the default face; 2. the outline faces outline-1, outline-2, ...
corresponding to the outline level.  So it's not clear what to prefer here.

But there is a simpler way to avoid such a problem: it's possible
to customize the face 'completions-group-separator' to remove
the face attribute 'strike-through'.  Then you can also use:
(setq-local outline-minor-mode-highlight 'override)
that will use the outline faces on the group headings,
and the button/margin will inherit it.





^ permalink raw reply	[flat|nested] 31+ messages in thread

* bug#57813: Icon images are non-functional
  2022-09-14 19:40 bug#57813: Icon images are non-functional Juri Linkov
  2022-09-15 16:30 ` Juri Linkov
@ 2022-10-13  7:51 ` Juri Linkov
  2022-10-18 18:14   ` Juri Linkov
  1 sibling, 1 reply; 31+ messages in thread
From: Juri Linkov @ 2022-10-13  7:51 UTC (permalink / raw)
  To: 57813

[-- Attachment #1: Type: text/plain, Size: 598 bytes --]

This patch distinguishes two cases of using buttons in outline-minor-mode:

1. when buttons can be inserted to a read-only buffer,
   then it's possible to move point to the inserted button
   and use it by keyboard;

2. when the buffer is editable and should not be modified
   to insert buttons, when the only way to display buttons
   is with a before-string overlay.  The disadvantage is that
   buttons can be used only by clicking mouse.

A new variable 'outline-read-only' is added here to allow
setting it explicitly in buffers that can be modified
to insert buttons like the Help buffer:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: outline-read-only.patch --]
[-- Type: text/x-diff, Size: 4277 bytes --]

diff --git a/lisp/help.el b/lisp/help.el
index b4b9120da3..713467c9b2 100644
--- a/lisp/help.el
+++ b/lisp/help.el
@@ -608,13 +608,8 @@ describe-bindings
           (setq-local outline-heading-end-regexp ":\n")
           (setq-local outline-level (lambda () 1))
           (setq-local outline-minor-mode-cycle t
-                      outline-minor-mode-highlight t)
+                      outline-minor-mode-highlight t
+                      outline-read-only t)
           (outline-minor-mode 1)
           (save-excursion
             (goto-char (point-min))
diff --git a/lisp/outline.el b/lisp/outline.el
index b87d3ac5e7..5a65c6c8e5 100644
--- a/lisp/outline.el
+++ b/lisp/outline.el
@@ -295,6 +295,9 @@ outline-minor-mode-use-buttons
 (defvar-local outline--use-buttons nil
   "Non-nil when buffer displays clickable buttons on the headings.")
 
+(defvar-local outline-read-only nil
+  "Non-nil when it's allowed to modify buffer to insert buttons.")
+
 (defvar-local outline--use-rtl nil
   "Non-nil when direction of clickable buttons is right-to-left.")
 
@@ -1652,18 +1655,24 @@ outline--make-button-overlay
                                    (if outline--use-rtl
                                        'outline-close-rtl
                                      'outline-close)
-                                 'outline-open)))
-          (inhibit-read-only t))
+                                 'outline-open))))
       ;; In editing buffers we use overlays only, but in other buffers
       ;; we use a mix of text properties, text and overlays to make
       ;; movement commands work more logically.
-      (when (derived-mode-p 'special-mode)
-        (put-text-property (point) (1+ (point)) 'face (plist-get icon 'face)))
-      (if-let ((image (plist-get icon 'image)))
-          (overlay-put o 'display image)
-        (overlay-put o 'display (concat (plist-get icon 'string)
-                                        (string (char-after (point)))))
-        (overlay-put o 'face (plist-get icon 'face))))
+      (if outline-read-only
+          (let ((inhibit-read-only t))
+            (put-text-property (point) (1+ (point)) 'face (plist-get icon 'face))
+            (if-let ((image (plist-get icon 'image)))
+                (overlay-put o 'display image)
+              (overlay-put o 'display (concat (plist-get icon 'string)
+                                              (string (char-after (point)))))
+              (overlay-put o 'face (plist-get icon 'face))))
+        (overlay-put
+         o 'before-string
+         (propertize " "
+                     'display
+                     (or (plist-get icon 'image)
+                         (plist-get icon 'string))))))
     o))
 
 (defun outline--make-margin-overlay (type)
@@ -1699,7 +1713,7 @@ outline--insert-open-button
       (beginning-of-line)
       (if use-margins
           (outline--make-margin-overlay 'open)
-        (when (derived-mode-p 'special-mode)
+        (when outline-read-only
           (let ((inhibit-read-only t))
             (insert "  ")
             (beginning-of-line)))
@@ -1716,7 +1730,7 @@ outline--insert-close-button
       (beginning-of-line)
       (if use-margins
           (outline--make-margin-overlay 'close)
-        (when (derived-mode-p 'special-mode)
+        (when outline-read-only
           (let ((inhibit-read-only t))
             (insert "  ")
             (beginning-of-line)))
diff --git a/lisp/textmodes/emacs-news-mode.el b/lisp/textmodes/emacs-news-mode.el
index d9decae4df..f7f56eb047 100644
--- a/lisp/textmodes/emacs-news-mode.el
+++ b/lisp/textmodes/emacs-news-mode.el
@@ -73,11 +73,7 @@ emacs-news-mode-font-lock-keywords
 
 (defun emacs-news--mode-common ()
   (setq-local font-lock-defaults '(emacs-news-mode-font-lock-keywords t))
-  ;; This `outline-regexp' matches leading spaces inserted
-  ;; by the current implementation of `outline-minor-mode-use-buttons'.
-  (setq-local outline-regexp "\\(?: +\\)?\\(\\*+\\) "
-              outline-level (lambda () (length (match-string 1)))
-              outline-minor-mode-cycle t
+  (setq-local outline-minor-mode-cycle t
               outline-minor-mode-highlight 'append)
   (outline-minor-mode)
   (setq-local imenu-generic-expression outline-imenu-generic-expression)

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* bug#57813: Icon images are non-functional
  2022-10-13  7:51 ` Juri Linkov
@ 2022-10-18 18:14   ` Juri Linkov
  2022-10-18 18:35     ` Juri Linkov
  0 siblings, 1 reply; 31+ messages in thread
From: Juri Linkov @ 2022-10-18 18:14 UTC (permalink / raw)
  To: 57813

> This patch distinguishes two cases of using buttons in outline-minor-mode:
>
> 1. when buttons can be inserted to a read-only buffer,
>    then it's possible to move point to the inserted button
>    and use it by keyboard;
>
> 2. when the buffer is editable and should not be modified
>    to insert buttons, when the only way to display buttons
>    is with a before-string overlay.  The disadvantage is that
>    buttons can be used only by clicking mouse.
>
> A new variable 'outline-read-only' is added here to allow
> setting it explicitly in buffers that can be modified
> to insert buttons like the Help buffer:

Pushed with the new variable named outline-minor-mode-insert-buttons.





^ permalink raw reply	[flat|nested] 31+ messages in thread

* bug#57813: Icon images are non-functional
  2022-10-18 18:14   ` Juri Linkov
@ 2022-10-18 18:35     ` Juri Linkov
  2022-10-19  6:44       ` Juri Linkov
  0 siblings, 1 reply; 31+ messages in thread
From: Juri Linkov @ 2022-10-18 18:35 UTC (permalink / raw)
  To: 57813

I wonder why outline-minor-mode-use-buttons is defcustom with a list of
modes.  Usually outline-minor-mode is parameterized by buffer-local
variables set before calling outline-minor-mode, for example:

  (setq-local outline-minor-mode-cycle t
              outline-minor-mode-highlight t
              outline-minor-mode-insert-buttons t)
   (outline-minor-mode)

and

  (setq-local outline-minor-mode-cycle t
              outline-minor-mode-highlight 'append
              outline-minor-mode-use-margins t)
   (outline-minor-mode)

So shouldn't outline-minor-mode-use-buttons be just a buffer-local
variable like all other variables that affect outline-minor-mode?
Then it could be set in mode hooks like other variables:

  (add-hook 'ruby-mode-hook
            (lambda ()
              (setq-local outline-default-state 2
                          outline-minor-mode-use-buttons t)
              (outline-minor-mode)))

Also outline-minor-mode-use-buttons could be set in dir/file-local
variables that has no problem with early init as in major modes.





^ permalink raw reply	[flat|nested] 31+ messages in thread

* bug#57813: Icon images are non-functional
  2022-10-18 18:35     ` Juri Linkov
@ 2022-10-19  6:44       ` Juri Linkov
  2022-10-22 18:38         ` Juri Linkov
  0 siblings, 1 reply; 31+ messages in thread
From: Juri Linkov @ 2022-10-19  6:44 UTC (permalink / raw)
  To: 57813

[-- Attachment #1: Type: text/plain, Size: 449 bytes --]

> So shouldn't outline-minor-mode-use-buttons be just a buffer-local
> variable like all other variables that affect outline-minor-mode?
> Then it could be set in mode hooks like other variables:
>
>   (add-hook 'ruby-mode-hook
>             (lambda ()
>               (setq-local outline-default-state 2
>                           outline-minor-mode-use-buttons t)
>               (outline-minor-mode)))

Here is a patch for this simplification:


[-- Attachment #2: outline-minor-mode-use-buttons.patch --]
[-- Type: text/x-diff, Size: 10778 bytes --]

diff --git a/lisp/help.el b/lisp/help.el
index 0f5342b77d..d48b866938 100644
--- a/lisp/help.el
+++ b/lisp/help.el
@@ -745,7 +745,7 @@ describe-bindings
           (setq-local outline-level (lambda () 1))
           (setq-local outline-minor-mode-cycle t
                       outline-minor-mode-highlight t
-                      outline-minor-mode-insert-buttons t)
+                      outline-minor-mode-use-buttons 'insert)
           (outline-minor-mode 1)
           (save-excursion
             (goto-char (point-min))
diff --git a/lisp/textmodes/emacs-news-mode.el b/lisp/textmodes/emacs-news-mode.el
index d57d053a7a..ebb31da9cf 100644
--- a/lisp/textmodes/emacs-news-mode.el
+++ b/lisp/textmodes/emacs-news-mode.el
@@ -75,7 +75,7 @@ emacs-news--mode-common
   (setq-local font-lock-defaults '(emacs-news-mode-font-lock-keywords t))
   (setq-local outline-minor-mode-cycle t
               outline-minor-mode-highlight 'append
-              outline-minor-mode-use-margins t)
+              outline-minor-mode-use-buttons 'in-margins)
   (outline-minor-mode)
   (setq-local imenu-generic-expression outline-imenu-generic-expression)
   (emacs-etc--hide-local-variables))
diff --git a/lisp/outline.el b/lisp/outline.el
index 2209964577..384ad6f2e7 100644
--- a/lisp/outline.el
+++ b/lisp/outline.el
@@ -281,40 +281,22 @@ outline-font-lock-faces
   [outline-1 outline-2 outline-3 outline-4
    outline-5 outline-6 outline-7 outline-8])
 
-(defcustom outline-minor-mode-use-buttons '(derived-mode . help-mode)
+(defvar-local outline-minor-mode-use-buttons nil
   "Whether to display clickable buttons on the headings.
-The value should be a `buffer-match-p' condition.
-
 These buttons can be used to hide and show the body under the heading.
-Note that this feature is not meant to be used in editing
-buffers (yet) -- that will be amended in a future version."
-  :type 'buffer-predicate
-  :safe #'booleanp
-  :version "29.1")
-
-(defvar-local outline--use-buttons nil
-  "Non-nil when buffer displays clickable buttons on the headings.")
-
-(defvar-local outline-minor-mode-insert-buttons nil
-  "Non-nil when it's allowed to modify buffer to insert buttons.")
+When the value is `insert', additional placeholders for buttons are
+inserted to the buffer, so buttons are not only clickable,
+but also typing `RET' on them can hide and show the body.
+When the value is `in-margins', then clickable buttons are
+displayed in the margins before the headings.
+When the value is `t', clickable buttons are displayed
+in the buffer before the headings.  The values `t' and
+`in-margins' can be used in editing buffers because they
+don't modify the buffer.")
 
 (defvar-local outline--use-rtl nil
   "Non-nil when direction of clickable buttons is right-to-left.")
 
-(defcustom outline-minor-mode-use-margins '(and (derived-mode . special-mode)
-                                                (not (derived-mode . help-mode)))
-  "Whether to display clickable buttons in the margins.
-The value should be a `buffer-match-p' condition.
-
-These buttons can be used to hide and show the body under the heading.
-Note that this feature is meant to be used in editing buffers."
-  :type 'buffer-predicate
-  :safe #'booleanp
-  :version "29.1")
-
-(defvar-local outline--use-margins nil
-  "Non-nil when buffer displays clickable buttons in the margins.")
-
 (define-icon outline-open nil
   '((image "outline-open.svg" "outline-open.pbm" :height (0.8 . em))
     (emoji "🔽")
@@ -487,7 +469,7 @@ outline-minor-mode-highlight-buffer
     (let ((regexp (concat "^\\(?:" outline-regexp "\\).*$")))
       (while (re-search-forward regexp nil t)
         (let ((overlay (make-overlay (match-beginning 0) (match-end 0))))
-          (overlay-put overlay 'outline-overlay t)
+          (overlay-put overlay 'outline-highlight t)
           ;; FIXME: Is it possible to override all underlying face attributes?
           (when (or (memq outline-minor-mode-highlight '(append override))
                     (and (eq outline-minor-mode-highlight t)
@@ -511,25 +493,19 @@ outline-minor-mode
             (key-description outline-minor-mode-prefix) outline-mode-prefix-map)
   (if outline-minor-mode
       (progn
-        (cond
-         ((buffer-match-p outline-minor-mode-use-margins (current-buffer))
-          (setq-local outline--use-margins t))
-         ((buffer-match-p outline-minor-mode-use-buttons (current-buffer))
-          (setq-local outline--use-buttons t)))
-        (when (and (or outline--use-buttons outline--use-margins)
-                   (eq (current-bidi-paragraph-direction) 'right-to-left))
-          (setq-local outline--use-rtl t))
-        (when outline--use-margins
-          (if outline--use-rtl
-              (setq-local right-margin-width (1+ right-margin-width))
-            (setq-local left-margin-width (1+ left-margin-width)))
-          (setq-local fringes-outside-margins t)
-          ;; Force display of margins
-          (when (eq (current-buffer) (window-buffer))
-            (set-window-buffer nil (window-buffer))))
-        (when (or outline--use-buttons outline--use-margins)
+        (when outline-minor-mode-use-buttons
           (add-hook 'after-change-functions
-                    #'outline--fix-buttons-after-change nil t))
+                    #'outline--fix-buttons-after-change nil t)
+          (when (eq (current-bidi-paragraph-direction) 'right-to-left)
+            (setq-local outline--use-rtl t))
+          (when (eq outline-minor-mode-use-buttons 'in-margins)
+            (if outline--use-rtl
+                (setq-local right-margin-width (1+ right-margin-width))
+              (setq-local left-margin-width (1+ left-margin-width)))
+            (setq-local fringes-outside-margins t)
+            ;; Force display of margins
+            (when (eq (current-buffer) (window-buffer))
+              (set-window-buffer nil (window-buffer)))))
         (when outline-minor-mode-highlight
           (if (and global-font-lock-mode (font-lock-specified-p major-mode))
               (progn
@@ -554,18 +530,18 @@ outline-minor-mode
       (if font-lock-fontified
           (font-lock-remove-keywords nil outline-font-lock-keywords))
       (font-lock-flush)
-      (remove-overlays nil nil 'outline-overlay t))
-    (when outline--use-buttons
-      (remove-overlays nil nil 'outline-button t))
-    (when outline--use-margins
-      (remove-overlays nil nil 'outline-margin t)
-      (if outline--use-rtl
-          (setq-local right-margin-width (1- right-margin-width))
-        (setq-local left-margin-width (1- left-margin-width)))
-      (setq-local fringes-outside-margins nil)
-      ;; Force removal of margins
-      (when (eq (current-buffer) (window-buffer))
-        (set-window-buffer nil (window-buffer))))))
+      (remove-overlays nil nil 'outline-highlight t))
+    (when outline-minor-mode-use-buttons
+      (if (not (eq outline-minor-mode-use-buttons 'in-margins))
+          (remove-overlays nil nil 'outline-button t)
+        (remove-overlays nil nil 'outline-margin t)
+        (if outline--use-rtl
+            (setq-local right-margin-width (1- right-margin-width))
+          (setq-local left-margin-width (1- left-margin-width)))
+        (setq-local fringes-outside-margins nil)
+        ;; Force removal of margins
+        (when (eq (current-buffer) (window-buffer))
+          (set-window-buffer nil (window-buffer)))))))
 
 (defvar-local outline-heading-alist ()
   "Alist associating a heading for every possible level.
@@ -1675,7 +1651,7 @@ outline--make-button-overlay
       ;; In editing buffers we use overlays only, but in other buffers
       ;; we use a mix of text properties, text and overlays to make
       ;; movement commands work more logically.
-      (if outline-minor-mode-insert-buttons
+      (if (eq outline-minor-mode-use-buttons 'insert)
           (let ((inhibit-read-only t))
             (put-text-property (point) (1+ (point)) 'face (plist-get icon 'face))
             (if-let ((image (plist-get icon 'image)))
@@ -1713,13 +1689,13 @@ outline--make-margin-overlay
                           (plist-get icon 'string))))))
     o))
 
-(defun outline--insert-open-button (&optional use-margins)
+(defun outline--insert-open-button (&optional in-margins)
   (with-silent-modifications
     (save-excursion
       (beginning-of-line)
-      (if use-margins
+      (if in-margins
           (outline--make-margin-overlay 'open)
-        (when outline-minor-mode-insert-buttons
+        (when (eq outline-minor-mode-use-buttons 'insert)
           (let ((inhibit-read-only t))
             (insert "  ")
             (beginning-of-line)))
@@ -1730,13 +1706,13 @@ outline--insert-open-button
                          "RET" #'outline-hide-subtree
                          "<mouse-2>" #'outline-hide-subtree)))))))
 
-(defun outline--insert-close-button (&optional use-margins)
+(defun outline--insert-close-button (&optional in-margins)
   (with-silent-modifications
     (save-excursion
       (beginning-of-line)
-      (if use-margins
+      (if in-margins
           (outline--make-margin-overlay 'close)
-        (when outline-minor-mode-insert-buttons
+        (when (eq outline-minor-mode-use-buttons 'insert)
           (let ((inhibit-read-only t))
             (insert "  ")
             (beginning-of-line)))
@@ -1748,7 +1724,7 @@ outline--insert-close-button
                          "<mouse-2>" #'outline-show-subtree)))))))
 
 (defun outline--fix-up-all-buttons (&optional from to)
-  (when (or outline--use-buttons outline--use-margins)
+  (when outline-minor-mode-use-buttons
     (when from
       (save-excursion
         (goto-char from)
@@ -1759,17 +1735,16 @@ outline--fix-up-all-buttons
              (outline-end-of-heading)
              (seq-some (lambda (o) (eq (overlay-get o 'invisible) 'outline))
                        (overlays-at (point))))
-           (outline--insert-close-button outline--use-margins)
-         (outline--insert-open-button outline--use-margins)))
+           (outline--insert-close-button (eq outline-minor-mode-use-buttons 'in-margins))
+         (outline--insert-open-button (eq outline-minor-mode-use-buttons 'in-margins))))
      (or from (point-min)) (or to (point-max)))))
 
 (defun outline--fix-buttons-after-change (beg end _len)
   ;; Handle whole lines
   (save-excursion (goto-char beg) (setq beg (pos-bol)))
   (save-excursion (goto-char end) (setq end (pos-eol)))
-  (when outline--use-buttons
-    (remove-overlays beg end 'outline-button t))
-  (when outline--use-margins
+  (if (not (eq outline-minor-mode-use-buttons 'in-margins))
+      (remove-overlays beg end 'outline-button t)
     (remove-overlays beg end 'outline-margin t))
   (outline--fix-up-all-buttons beg end))
 

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* bug#57813: Icon images are non-functional
  2022-10-19  6:44       ` Juri Linkov
@ 2022-10-22 18:38         ` Juri Linkov
  0 siblings, 0 replies; 31+ messages in thread
From: Juri Linkov @ 2022-10-22 18:38 UTC (permalink / raw)
  To: 57813

close 57813 29.0.50
thanks

> Here is a patch for this simplification:

Now pushed and closed.





^ permalink raw reply	[flat|nested] 31+ messages in thread

* bug#57813: Icon images are non-functional
  2022-10-12 18:55             ` Juri Linkov
@ 2022-10-23 16:55               ` Juri Linkov
  0 siblings, 0 replies; 31+ messages in thread
From: Juri Linkov @ 2022-10-23 16:55 UTC (permalink / raw)
  To: Max Brieiev; +Cc: Lars Ingebrigtsen, 57813

>> It seems in some cases this can cause an overlap between the heading and
>> the image, like on the screenshot:
>
> It's hard to guess whether the outlines are using buttons or margins.
> Probably buttons, because margins are not displayed in *Completions*.
> Actually the same problem exists for both of them.  By default the
> button/margin face is inherited, because in most modes inheriting
> fontification from the outline heading lines provides a nicer look,
> but your example demonstrates that in some cases that face needs to be
> overridden.  Then there are several variants what face to use instead:
> 1. the default face; 2. the outline faces outline-1, outline-2, ...
> corresponding to the outline level.  So it's not clear what to prefer here.

Now refactored the outline button icon functions to support
the face attribute of the icon for margins and buttons.
So if an icon definition is customized to contain
e.g. ':face default':

  (define-icon outline-open nil
    '((image "outline-open.svg" "outline-open.pbm" :height (0.8 . em)
       :face default)
      (symbol " ▼ "
       :face default))

then it doesn't inherit the outline heading face.

> But there is a simpler way to avoid such a problem: it's possible
> to customize the face 'completions-group-separator' to remove
> the face attribute 'strike-through'.  Then you can also use:
> (setq-local outline-minor-mode-highlight 'override)
> that will use the outline faces on the group headings,
> and the button/margin will inherit it.

For better compatibility with different face combinations,
I changed the 'completions-group-separator' face attribute
'strike-through' to 'underline'.





^ permalink raw reply	[flat|nested] 31+ messages in thread

end of thread, other threads:[~2022-10-23 16:55 UTC | newest]

Thread overview: 31+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-09-14 19:40 bug#57813: Icon images are non-functional Juri Linkov
2022-09-15 16:30 ` Juri Linkov
2022-09-15 16:36   ` Juri Linkov
2022-09-15 16:53     ` Eli Zaretskii
2022-09-15 17:35       ` Juri Linkov
2022-09-15 18:32         ` Eli Zaretskii
2022-09-15 19:46           ` Juri Linkov
2022-09-16 16:16             ` Juri Linkov
2022-09-15 19:53   ` Juri Linkov
2022-09-16  5:29     ` Eli Zaretskii
2022-09-16  7:09       ` Juri Linkov
2022-09-17 19:37         ` Juri Linkov
2022-09-18  5:08           ` Eli Zaretskii
2022-09-18 18:46             ` Juri Linkov
2022-09-18  5:17           ` Eli Zaretskii
2022-09-18 19:06             ` Juri Linkov
2022-09-19 12:12               ` Eli Zaretskii
2022-09-19 19:37                 ` Juri Linkov
2022-09-20 11:31                   ` Eli Zaretskii
2022-09-20 16:12                     ` Juri Linkov
2022-09-23 15:48     ` Juri Linkov
2022-09-23 16:26       ` Lars Ingebrigtsen
2022-09-24 17:17         ` Juri Linkov
2022-10-12 14:42           ` Max Brieiev
2022-10-12 18:55             ` Juri Linkov
2022-10-23 16:55               ` Juri Linkov
2022-10-13  7:51 ` Juri Linkov
2022-10-18 18:14   ` Juri Linkov
2022-10-18 18:35     ` Juri Linkov
2022-10-19  6:44       ` Juri Linkov
2022-10-22 18:38         ` Juri Linkov

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).