[ELPA] New packages: boxy, boxy-headlines

[ELPA] New packages: boxy, boxy-headlines

[Tyler Grinn]

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


I would like to propose two new packages for ELPA.

Boxy provides a framework for creating a boxy layout. Each box has a
relationship with one other box. A relationship can be either in, on,
behind, in front of, on top of, above, below, to the left of, or to the
right of. A box may have multiple boxes related to it. In a boxy
diagram, standard emacs movement keys navigate by boxes and each box can be
expanded or collapsed.

https://gitlab.com/tygrdev/boxy

An example application is boxy-headlines, which provides the command
boxy-headlines, which views the current org file as a boxy diagram with
each headline represented by a box. The relationship between a parent
and child headline can be configured by modifying the REL property of
the child headline. The tooltip shows the values that would be displayed
for the headline in columns view.

https://gitlab.com/tygrdev/boxy-headlines

Another application is org-real, which lets you keep track of real
things as org-mode links. Because org-real was written first, it doesn't
depend on boxy in its current state. Once boxy is available on ELPA,
I'll add it as a dependency and submit org-real to ELPA.

https://gitlab.com/tygrdev/org-real/-/tree/next



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Patch for adding boxy and boxy headlines --]
[-- Type: text/x-diff, Size: 1071 bytes --]

From 00ef851758ab54677f5e60aab3570e547933553d Mon Sep 17 00:00:00 2001
From: Tyler Grinn <tylergrinn@gmail.com>
Date: Mon, 4 Oct 2021 14:37:47 -0400
Subject: [PATCH] Add boxy and boxy-headlines

boxy is an engine for creating boxy layouts. An example application is
boxy-headlines, which shows all headlines in the current org file as a
boxy diagram.
---
 elpa-packages | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/elpa-packages b/elpa-packages
index 5c9fb53133..480f7dd2a8 100644
--- a/elpa-packages
+++ b/elpa-packages
@@ -59,6 +59,12 @@
  ("beacon"		:url "https://github.com/Malabarba/beacon")
  ("bluetooth"		:url "https://gitlab.com/rstocker/emacs-bluetooth")
  ("bnf-mode"		:url "https://github.com/sergeyklay/bnf-mode")
+ ("boxy"
+  :url "https://gitlab.com/tygrdev/boxy"
+  :auto-sync t)
+ ("boxy-headlines"
+  :url "https://gitlab.com/tygrdev/boxy-headlines"
+  :auto-sync t)
  ("brief"		:url nil)
  ("buffer-expose"	:url "https://github.com/clemera/buffer-expose")
  ("bug-hunter"		:url "https://github.com/Malabarba/elisp-bug-hunter")
-- 
2.30.2


[-- Attachment #3: Type: text/plain, Size: 15 bytes --]



Best,

Tyler

Re: [ELPA] New packages: boxy, boxy-headlines

[Stefan Monnier]

> https://gitlab.com/tygrdev/boxy
> https://gitlab.com/tygrdev/boxy-headlines

I just pushed them to elpa.git.
Some comments about the code:

- Why use `face-spec-set` instead of putting the spec directly in the
  `defface`?

- Your packages have no ;;;###autoload cookie.  The doc of boxy.el
  doesn't make it clear what's a likely entry point (i.e. how to start
  using it), but for `boxy-headings`, it seems at least the
  `boxy-headlines` function should have such a cookie.

- When taking contributions from other people, please make sure they
  have signed the needed paperwork *before* rather than after accepting
  the contribution.

See below a patch resulting compiling the code and looking around.


        Stefan


diff --git a/boxy.el b/boxy.el
index f293228c03..b82a5e3db9 100644
--- a/boxy.el
+++ b/boxy.el
@@ -123,58 +123,47 @@
 
 (defcustom boxy-default-margin-x 2
   "Default horizontal margin to be used when displaying boxes."
-  :type 'number
-  :group 'boxy)
+  :type 'number)
 
 (defcustom boxy-default-margin-y 1
   "Default vertical margin to be used when displaying boxes."
-  :type 'number
-  :group 'boxy)
+  :type 'number)
 
 (defcustom boxy-default-padding-x 2
   "Default horizontal padding to be used when displaying boxes."
-  :type 'number
-  :group 'boxy)
+  :type 'number)
 
 (defcustom boxy-default-padding-y 1
   "Default vertical padding to be used when displaying boxes."
-  :type 'number
-  :group 'boxy)
+  :type 'number)
 
 (defcustom boxy-flex-width 80
   "When flexibly displaying boxes, try to keep width below this."
-  :type 'number
-  :group 'boxy)
+  :type 'number)
 
 (defcustom boxy-default-visibility 2
   "Default level to display boxes."
-  :type 'number
-  :group 'boxy)
+  :type 'number)
 
 (defcustom boxy-tooltips t
   "Show tooltips in a boxy diagram."
-  :type 'boolean
-  :group 'boxy)
+  :type 'boolean)
 
 (defcustom boxy-tooltip-timeout 0.5
   "Idle time before showing tooltip in a boxy diagram."
-  :type 'number
-  :group 'boxy)
+  :type 'number)
 
 (defcustom boxy-tooltip-max-width 30
   "Maximum width of all tooltips."
-  :type 'number
-  :group 'boxy)
+  :type 'number)
 
 ;;;; Faces
 
 (defface boxy-default nil
-  "Default face used in Boxy mode."
-  :group 'boxy)
+  "Default face used in Boxy mode.")
 
 (defface boxy-primary nil
-  "Face for highlighting the name of a box."
-  :group 'boxy)
+  "Face for highlighting the name of a box.")
 
 (face-spec-set
  'boxy-primary
@@ -183,8 +172,7 @@
  'face-defface-spec)
 
 (defface boxy-selected nil
-  "Face for the current box border under cursor."
-  :group 'boxy)
+  "Face for the current box border under cursor.")
 
 (face-spec-set
  'boxy-selected
@@ -192,8 +180,7 @@
  'face-defface-spec)
 
 (defface boxy-rel nil
-  "Face for the box which is related to the box under the cursor."
-  :group 'boxy)
+  "Face for the box which is related to the box under the cursor.")
 
 (face-spec-set
  'boxy-rel
@@ -201,8 +188,7 @@
  'face-defface-spec)
 
 (defface boxy-tooltip nil
-  "Face for tooltips in a boxy diagram."
-  :group 'boxy)
+  "Face for tooltips in a boxy diagram.")
 
 (face-spec-set
  'boxy-tooltip
@@ -393,11 +379,11 @@
   "Recalculate the position of all boxes in `boxy--boxes'."
   (setq boxy--box-ring
         (seq-sort
-         '<
+         #'<
          (seq-filter
-          'identity
+          #'identity
           (mapcar
-           'boxy--get-position
+           #'boxy--get-position
            (seq-filter
             (lambda (box) (boxy-is-visible box t))
             boxy--boxes))))))
@@ -418,38 +404,35 @@
      (if (slot-boundp box :height) (slot-makeunbound box :height)))
    boxy--boxes))
 
+(defvar boxy-mode-map
+  (let ((map (make-sparse-keymap)))
+    (mapc (lambda (key) (define-key map (kbd (car key)) (cdr key)))
+          '(("TAB"       . boxy-mode-cycle)
+            ("<right>"   . boxy-mode-cycle)
+            ("C-f"       . boxy-mode-cycle)
+            ("M-f"       . boxy-mode-cycle)
+            ("f"         . boxy-mode-cycle)
+            ("<left>"    . boxy-mode-uncycle)
+            ("C-b"       . boxy-mode-uncycle)
+            ("M-b"       . boxy-mode-uncycle)
+            ("b"         . boxy-mode-uncycle)
+            ("<up>"      . boxy-mode-cycle-up)
+            ("C-p"       . boxy-mode-cycle-up)
+            ("p"         . boxy-mode-cycle-up)
+            ("<down>"    . boxy-mode-cycle-down)
+            ("C-n"       . boxy-mode-cycle-down)
+            ("n"         . boxy-mode-cycle-down)
+            ("<backtab>" . boxy-mode-cycle-visibility)))
+    map))
+
 (define-derived-mode boxy-mode special-mode
   "Boxy"
-  "Mode for viewing an boxy diagram.
-
-The following commands are available:
-
-\\{boxy-mode-map}"
-  (let ((inhibit-message t))
+  "Mode for viewing an boxy diagram."
+  (let ((inhibit-message t))     ;FIXME: Please report the message as an error.
     (setq indent-tabs-mode nil)
     (cursor-sensor-mode t)
     (toggle-truncate-lines t)))
 
-(mapc
- (lambda (key) (define-key boxy-mode-map (kbd (car key)) (cdr key)))
- '(("TAB"       . boxy-mode-cycle)
-   ("<right>"   . boxy-mode-cycle)
-   ("C-f"       . boxy-mode-cycle)
-   ("M-f"       . boxy-mode-cycle)
-   ("f"         . boxy-mode-cycle)
-   ("<left>"    . boxy-mode-uncycle)
-   ("C-b"       . boxy-mode-uncycle)
-   ("M-b"       . boxy-mode-uncycle)
-   ("b"         . boxy-mode-uncycle)
-   ("<up>"      . boxy-mode-cycle-up)
-   ("C-p"       . boxy-mode-cycle-up)
-   ("p"         . boxy-mode-cycle-up)
-   ("<down>"    . boxy-mode-cycle-down)
-   ("C-n"       . boxy-mode-cycle-down)
-   ("n"         . boxy-mode-cycle-down)
-   ("<backtab>" . boxy-mode-cycle-visibility)))
-
-
 (cl-defun boxy-pp (box
                    &key
                    (display-buffer-fn 'display-buffer-pop-up-window)
@@ -528,7 +511,7 @@ diagram."
       (setq boxy--selected-face selected-face)
       (boxy-mode-update-visibility)
       (boxy-mode-redraw)
-      (let* ((width (apply 'max (mapcar 'length (split-string (buffer-string) "\n"))))
+      (let* ((width (apply #'max (mapcar #'length (split-string (buffer-string) "\n"))))
              (height (count-lines (point-min) (point-max)))
              (window (or (get-buffer-window buffer)
                          (display-buffer buffer
@@ -1001,10 +984,10 @@ Uses `boxy--offset' to determine row and column offsets."
                 (setq r (+ r 1))))))))
     (if border-face
         (if box-coords (list box-coords) nil)
-      (apply 'append
+      (apply #'append
              (if box-coords (list box-coords) nil)
              (mapcar
-              'boxy-draw
+              #'boxy-draw
               (boxy--get-children box))))))
 
 (cl-defmethod boxy--get-width ((box boxy-box))
@@ -1014,52 +997,54 @@ Uses `boxy--offset' to determine row and column offsets."
         stored-width
       (let* ((margin (boxy--margin-x box))
              (padding (boxy--padding-x box))
-             (base-width (+ 2 ; box walls
+             (base-width (+ 2           ; box walls
                             (* 2 padding)))
              (width (+ base-width
                        (if (slot-boundp box :name)
                            (with-slots (name) box (length name))
                          0)))
              (children (boxy--get-children box)))
-        (if (not children)
-            (setq stored-width width)
-          (let* ((row-indices (cl-delete-duplicates
-                               (mapcar
-                                (lambda (child) (with-slots (y-order) child y-order))
-                                children)))
-                 (rows (mapcar
-                        (lambda (r)
-                          (cl-delete-duplicates
-                           (seq-filter
-                            (lambda (child) (with-slots (y-order) child (= r y-order)))
-                            children)
-                           :test #'(lambda (a b)
-                                     (and (slot-boundp a :name)
-                                          (slot-boundp b :name)
-                                          (string= (with-slots (name) a name)
-                                                   (with-slots (name) b name))))))
-                        row-indices))
-                 (children-width (apply 'max
-                                        (mapcar
-                                         (lambda (row)
-                                           (seq-reduce
-                                            (lambda (sum width)
-                                              (+ sum width margin))
-                                            (mapcar 'boxy--get-width row)
-                                            (* -1 margin)))
-                                         rows))))
-            (if (> width (+ (* 2 padding) children-width))
-                (setq stored-width width)
-              (setq stored-width (+ base-width children-width)))))))))
+        (setq stored-width
+              (if (not children)
+                  width
+                (let* ((row-indices (cl-delete-duplicates
+                                     (mapcar
+                                      (lambda (child) (with-slots (y-order) child y-order))
+                                      children)))
+                       (rows (mapcar
+                              (lambda (r)
+                                (cl-delete-duplicates
+                                 (seq-filter
+                                  (lambda (child) (with-slots (y-order) child (= r y-order)))
+                                  children)
+                                 :test #'(lambda (a b)
+                                           (and (slot-boundp a :name)
+                                                (slot-boundp b :name)
+                                                (string= (with-slots (name) a name)
+                                                         (with-slots (name) b name))))))
+                              row-indices))
+                       (children-width (apply #'max
+                                              (mapcar
+                                               (lambda (row)
+                                                 (seq-reduce
+                                                  (lambda (sum width)
+                                                    (+ sum width margin))
+                                                  (mapcar #'boxy--get-width row)
+                                                  (* -1 margin)))
+                                               rows))))
+                  (if (> width (+ (* 2 padding) children-width))
+                      width
+                    (+ base-width children-width)))))))))
 
 (cl-defmethod boxy--get-on-top-height ((box boxy-box))
   "Get the height of any boxes on top of BOX."
-  (apply 'max 0
+  (apply #'max 0
          (mapcar
-          'boxy--get-on-top-height-helper
+          #'boxy--get-on-top-height-helper
           (seq-filter
-           (lambda (child) (with-slots (rel) child (and (slot-boundp child :rel)
-                                                        (string= rel "on top of"))))
+           (lambda (child) (with-slots (rel) child
+                        (and (slot-boundp child :rel)
+                             (string= rel "on top of"))))
            (boxy--get-children box)))))
 
 (cl-defmethod boxy--get-on-top-height-helper ((child boxy-box))
@@ -1067,9 +1052,9 @@ Uses `boxy--offset' to determine row and column offsets."
   (with-slots (rel) child
     (+
      (boxy--get-height child)
-     (apply 'max 0
+     (apply #'max 0
             (mapcar
-             'boxy--get-on-top-height-helper
+             #'boxy--get-on-top-height-helper
              (seq-filter
               (lambda (grandchild)
                 (with-slots ((grandchild-rel rel)) grandchild
@@ -1106,7 +1091,7 @@ If INCLUDE-ON-TOP is non-nil, also include height on top of box."
                                        (+ sum margin row))
                                      (mapcar
                                       (lambda (r)
-                                        (apply 'max 0
+                                        (apply #'max 0
                                                (mapcar
                                                 (lambda (child) (boxy--get-height child t))
                                                 (seq-filter
@@ -1153,7 +1138,7 @@ If INCLUDE-ON-TOP is non-nil, also include height on top of box."
                                             siblings
                                             '()))
                            (above-bottom (+ margin
-                                             (apply 'max
+                                             (apply #'max
                                                     (mapcar
                                                      (lambda (sibling)
                                                        (+ (boxy--get-top sibling)
@@ -1360,7 +1345,7 @@ BOX is the box the button is being made for."
     (if (not (boxy-is-visible box))
         (if children (cl-rotatef children hidden-children))
       (boxy--expand-box box))
-    (mapc 'boxy--update-visibility children)))
+    (mapc #'boxy--update-visibility children)))
 
 (cl-defmethod boxy--get-position ((box boxy-box))
   "Get the buffer position of the names of BOX and its children."
@@ -1443,19 +1428,19 @@ If FORCE-VISIBLE, always make CHILD visible in PARENT."
   "Get a list of boxes from BOX which have no further relatives."
   (if (slot-boundp box :parent)
       (if-let ((next-boxes (boxy--next box)))
-          (apply 'append (mapcar 'boxy--primary-boxes next-boxes))
+          (apply #'append (mapcar #'boxy--primary-boxes next-boxes))
         (list box))
-    (apply 'append (mapcar 'boxy--primary-boxes (boxy--get-children box 'all)))))
+    (apply #'append (mapcar #'boxy--primary-boxes (boxy--get-children box 'all)))))
 
 (cl-defmethod boxy--expand ((box boxy-box))
   "Get a list of all boxes, including BOX, that are related to BOX."
   (if (slot-boundp box :parent)
-      (apply 'append (list box) (mapcar 'boxy--expand (boxy--next box)))
-    (apply 'append (mapcar 'boxy--expand (boxy--get-children box 'all)))))
+      (apply #'append (list box) (mapcar #'boxy--expand (boxy--next box)))
+    (apply #'append (mapcar #'boxy--expand (boxy--get-children box 'all)))))
 
 (cl-defmethod boxy--get-all ((box boxy-box))
   "Get all boxes, including BOX, that are children of BOX."
-  (apply 'append (list box) (mapcar 'boxy--get-all (boxy--get-children box 'all))))
+  (apply #'append (list box) (mapcar #'boxy--get-all (boxy--get-children box 'all))))
 
 (cl-defmethod boxy--next ((box boxy-box) &optional exclude-children)
   "Retrieve any boxes for which the :rel-box slot is BOX.
@@ -1536,10 +1521,10 @@ If EXCLUDE-CHILDREN, only retrieve sibling boxes."
                                         (not (or in-front on-top))))
                                     (boxy--get-children parent)))))
             (if (string= rel "above")
-                (setq y-order (- (apply 'min 0 sibling-y-orders) 1))
-              (setq y-order (+ 1 (apply 'max 0 sibling-y-orders))))))
+                (setq y-order (- (apply #'min 0 sibling-y-orders) 1))
+              (setq y-order (+ 1 (apply #'max 0 sibling-y-orders))))))
          ((or on-top in-front)
-          (setq x-order (+ 1 (apply 'max 0
+          (setq x-order (+ 1 (apply #'max 0
                                     (mapcar
                                      (lambda (child) (with-slots (x-order) child x-order))
                                      (seq-filter
@@ -1656,7 +1641,7 @@ characters if possible."
          (rows (split-string content "\n"))
          (height (length rows))
          (width (+ 2 (min boxy--tooltip-max-width
-                          (apply 'max 0 (mapcar 'length rows)))))
+                          (apply #'max 0 (mapcar #'length rows)))))
          (top (if (< (- cur-line 2 height) min-line)
                   (+ cur-line 2)
                 (- cur-line 1 height)))
@@ -1689,7 +1674,7 @@ characters if possible."
         (setq top (+ top 1))))
     (save-excursion (boxy-mode-recalculate-box-ring))
     (push (read-event nil) unread-command-events)
-    (mapc 'delete-overlay overlays)))
+    (mapc #'delete-overlay overlays)))
 
 (provide 'boxy)
 
diff --git a/tests/boxy-test-setup.el b/tests/boxy-test-setup.el
index a6691388a2..9711ef8b83 100644
--- a/tests/boxy-test-setup.el
+++ b/tests/boxy-test-setup.el
@@ -6,7 +6,7 @@
 
 ;;; Code:
 
-(load-file "boxy.el")
+(require 'boxy "boxy.el")           ;FIXME: Why insist on loading the .el file?
 
 (setq boxy-default-margin-x 0)
 (setq boxy-default-margin-y 1)
diff --git a/boxy-headlines.el b/boxy-headlines.el
index 36548743e2..6c5328633d 100644
--- a/boxy-headlines.el
+++ b/boxy-headlines.el
@@ -59,63 +59,51 @@
 
 (defcustom boxy-headlines-margin-x 2
   "Horizontal margin to be used when displaying boxes."
-  :type 'number
-  :group 'boxy-headlines)
+  :type 'number)
 
 (defcustom boxy-headlines-margin-y 1
   "Vertical margin to be used when displaying boxes."
-  :type 'number
-  :group 'boxy-headlines)
+  :type 'number)
 
 (defcustom boxy-headlines-padding-x 2
   "Horizontal padding to be used when displaying boxes."
-  :type 'number
-  :group 'boxy-headlines)
+  :type 'number)
 
 (defcustom boxy-headlines-padding-y 1
   "Vertical padding to be used when displaying boxes."
-  :type 'number
-  :group 'boxy-headlines)
+  :type 'number)
 
 (defcustom boxy-headlines-include-context t
   "Whether to show context when opening a real link."
-  :type 'boolean
-  :group 'boxy-headlines)
+  :type 'boolean)
 
 (defcustom boxy-headlines-flex-width 80
   "When merging links, try to keep width below this."
-  :type 'number
-  :group 'boxy-headlines)
+  :type 'number)
 
 (defcustom boxy-headlines-default-visibility 1
   "Default level to display boxes."
-  :type 'number
-  :group 'boxy-headlines)
+  :type 'number)
 
 (defcustom boxy-headlines-tooltips t
   "Show tooltips in a boxy diagram."
-  :type 'boolean
-  :group 'boxy-headlines)
+  :type 'boolean)
 
 (defcustom boxy-headlines-tooltip-timeout 0.5
   "Idle time before showing tooltip in a boxy diagram."
-  :type 'number
-  :group 'boxy-headlines)
+  :type 'number)
 
 (defcustom boxy-headlines-tooltip-max-width 30
   "Maximum width of all tooltips."
-  :type 'number
-  :group 'boxy-headlines)
+  :type 'number)
 
 ;;;; Faces
 
 (defface boxy-headlines-default nil
-  "Default face used in boxy mode."
-  :group 'boxy-headlines)
+  "Default face used in boxy mode.")
 
 (defface boxy-headlines-primary nil
-  "Face for highlighting the name of a box."
-  :group 'boxy-headlines)
+  "Face for highlighting the name of a box.")
 
 (face-spec-set
  'boxy-headlines-primary
@@ -124,8 +112,7 @@
  'face-defface-spec)
 
 (defface boxy-headlines-selected nil
-  "Face for the current box border under cursor."
-  :group 'boxy-headlines)
+  "Face for the current box border under cursor.")
 
 (face-spec-set
  'boxy-headlines-selected
@@ -133,8 +120,7 @@
  'face-defface-spec)
 
 (defface boxy-headlines-rel nil
-  "Face for the box which is related to the box under the cursor."
-  :group 'boxy-headlines)
+  "Face for the box which is related to the box under the cursor.")
 
 (face-spec-set
  'boxy-headlines-rel
@@ -142,8 +128,7 @@
  'face-defface-spec)
 
 (defface boxy-headlines-tooltip nil
-  "Face for tooltips in a boxy diagram."
-  :group 'boxy-headlines)
+  "Face for tooltips in a boxy diagram.")
 
 (face-spec-set
  'boxy-headlines-tooltip
@@ -227,11 +212,12 @@ diagram."
 
 ;;;; Commands
 
+;;;###autoload
 (defun boxy-headlines ()
   "View all org headlines as a boxy diagram."
   (interactive)
   (let ((path (seq-filter
-               'identity
+               #'identity
                (append (list (org-entry-get nil "ITEM"))
                        (reverse (org-get-outline-path)))))
         (world (save-excursion (boxy-headlines--parse-headlines)))
@@ -266,7 +252,7 @@ diagram."
              (siblings (alist-get 'siblings partitioned))
              (pos (org-element-property :begin headline))
              (columns (save-excursion (goto-char pos) (org-columns--collect-values)))
-             (max-column-length (apply 'max 0
+             (max-column-length (apply #'max 0
                                        (mapcar
                                         (lambda (column)
                                           (length (cadr (car column))))

Re: [ELPA] New packages: boxy, boxy-headlines

[Stefan Monnier]

> Some comments about the code:

Oh, I forgot the main question (the one that got me looking more
closely in the first place): why do you use `cl-defmethod` everywhere
yet with only ever a single method defined under the same name?


        Stefan

Re: [ELPA] New packages: boxy, boxy-headlines

[Tyler Grinn]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> https://gitlab.com/tygrdev/boxy
>> https://gitlab.com/tygrdev/boxy-headlines
> 
> I just pushed them to elpa.git.
> Some comments about the code:
> 
> - Why use `face-spec-set` instead of putting the spec directly in the
>   `defface`?

This is my first package so I'm not sure I have good answers to all your
questions.

Defface wasn't working for me when updating the color scheme. Repeated
calls to defface don't seem to apply changes, while directly modifying
the face-defface-spec does.

> 
> - Your packages have no ;;;###autoload cookie.  The doc of boxy.el
>   doesn't make it clear what's a likely entry point (i.e. how to start
>   using it), but for `boxy-headings`, it seems at least the
>   `boxy-headlines` function should have such a cookie.

I think you're right about the boxy-headlines command. boxy.el provides
the boxy major mode and related commands, should those be autoloaded?

> - When taking contributions from other people, please make sure they
>   have signed the needed paperwork *before* rather than after accepting
>   the contribution.
>

I am the sole contributor to this project as of right now but I will
keep that in mind.

> Oh, I forgot the main question (the one that got me looking more
> closely in the first place): why do you use `cl-defmethod` everywhere
> yet with only ever a single method defined under the same name?
>

That was so (oref) and (oset) can work without ELC complaining about
'unknown slots'. Is there another macro that uses class specializers?


Also, why not assign custom variables and faces to a group?


Thank you for taking a look at the code and providing some feedback,
much appreciated.

Re: [ELPA] New packages: boxy, boxy-headlines

[Stefan Monnier]

> Defface wasn't working for me when updating the color scheme. Repeated
> calls to defface don't seem to apply changes, while directly modifying
> the face-defface-spec does.

AFAIK, same as for `defvar`, this is on-purpose.
If you want to re-apply the standard definition, the recommended way is
to use `C-M-x`.

>> - Your packages have no ;;;###autoload cookie.  The doc of boxy.el
>>   doesn't make it clear what's a likely entry point (i.e. how to start
>>   using it), but for `boxy-headings`, it seems at least the
>>   `boxy-headlines` function should have such a cookie.
>
> I think you're right about the boxy-headlines command. boxy.el provides
> the boxy major mode and related commands, should those be autoloaded?

IIUC `boxy.el` (including the major mode) is not meant to be used
directly by the end user, so maybe it doesn't need any autoloads because
the clients will just (require 'boxy) at the top of their file.
I don't know enough about how `boxy.el` is used by client packages:
you're in a better position to know.

>> Oh, I forgot the main question (the one that got me looking more
>> closely in the first place): why do you use `cl-defmethod` everywhere
>> yet with only ever a single method defined under the same name?
> That was so (oref) and (oset) can work without ELC complaining about
> 'unknown slots'. Is there another macro that uses class specializers?

Hmm... I don't understand why using `cl-defmethod` would silence
"unknown slot" warnings.  AFAIK those warnings depend solely on whether
the byte-compiler has seen the corresponding `defclass`.

> Also, why not assign custom variables and faces to a group?

Removing those `:group` will not stop them from being assigned to those
groups (IOW those `:group` args are simply redundant because vars and
faces are assigned (by default) to the last group defined with `defgroup`).


        Stefan

Re: [ELPA] New packages: boxy, boxy-headlines

[Tyler Grinn]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> Defface wasn't working for me when updating the color scheme. Repeated
>> calls to defface don't seem to apply changes, while directly modifying
>> the face-defface-spec does.
>
> AFAIK, same as for `defvar`, this is on-purpose.
> If you want to re-apply the standard definition, the recommended way is
> to use `C-M-x`.
>

That's what I've gathered to, so to get around that is why I'm using
face-spec-set. The behavior I want is this: update the face if a user
has not independently customized it already. If I simply changed the
defface, only new users would get the new theme. Existing users would
have to re-apply the standard definition or restart emacs.

>>> - Your packages have no ;;;###autoload cookie.  The doc of boxy.el
>>>   doesn't make it clear what's a likely entry point (i.e. how to start
>>>   using it), but for `boxy-headings`, it seems at least the
>>>   `boxy-headlines` function should have such a cookie.
>>
>> I think you're right about the boxy-headlines command. boxy.el provides
>> the boxy major mode and related commands, should those be autoloaded?
>
> IIUC `boxy.el` (including the major mode) is not meant to be used
> directly by the end user, so maybe it doesn't need any autoloads because
> the clients will just (require 'boxy) at the top of their file.
> I don't know enough about how `boxy.el` is used by client packages:
> you're in a better position to know.
>

Ok I agree, that is basic use case I was going for.

>>> Oh, I forgot the main question (the one that got me looking more
>>> closely in the first place): why do you use `cl-defmethod` everywhere
>>> yet with only ever a single method defined under the same name?
>> That was so (oref) and (oset) can work without ELC complaining about
>> 'unknown slots'. Is there another macro that uses class specializers?
>
> Hmm... I don't understand why using `cl-defmethod` would silence
> "unknown slot" warnings.  AFAIK those warnings depend solely on whether
> the byte-compiler has seen the corresponding `defclass`.
>

Ah that explains it. I thought it was necessary based on reading the
eieio documentation and I couldn't find any support for the unknown slot
warning. I must've reordered it at some point without realizing that was
what fixed it.

Maybe you can help with a related problem. I tried to define a macro
boxy--inherited:


(defmacro boxy--inherited (fun-name slot default)
  `(defun ,fun-name (box)
     ,(concat "Get the inherited property " (symbol-name slot) " from BOX")
     (if (slot-boundp box ,slot)
         (oref box ,slot)
       (if (slot-boundp box :parent)
           (,fun-name (oref box :parent))
         ,default))))

(boxy--inherited boxy--padding-y :padding-y boxy--default-padding-y)

But this results in
Warning: Unknown slot ‘:padding-y’
Warning: Unknown slot ‘:parent’


>> Also, why not assign custom variables and faces to a group?
>
> Removing those `:group` will not stop them from being assigned to those
> groups (IOW those `:group` args are simply redundant because vars and
> faces are assigned (by default) to the last group defined with `defgroup`).
>
>
>         Stefan

That's neat!

I have two more questions about the patch. First, just to make sure,
would you like to contribute those three diffs and did you sign the FSF
Copyright paperwork?


> +  (let ((inhibit-message t))     ;FIXME: Please report the message as an error.
>      (setq indent-tabs-mode nil)
>      (cursor-sensor-mode t)
>      (toggle-truncate-lines t)))
>

This doesn't even do what I want. I don't want 'Truncate long lines
enabled' to be printed to the minibuffer when entering boxy mode, but it
shows regardless. What exactly does the FIXME mean here and how do I
stop that message?

Re: [ELPA] New packages: boxy, boxy-headlines

[Stefan Monnier]

> That's what I've gathered to, so to get around that is why I'm using
> face-spec-set. The behavior I want is this: update the face if a user
> has not independently customized it already. If I simply changed the
> defface, only new users would get the new theme. Existing users would
> have to re-apply the standard definition or restart emacs.

What you want is not specific to your theme, so it's best to
fix it at its source rather than work around it with non-standard code.
I suggest you `M-x report-emacs-bug` and request this behavior.

> (boxy--inherited boxy--padding-y :padding-y boxy--default-padding-y)
>
> But this results in
> Warning: Unknown slot ‘:padding-y’
> Warning: Unknown slot ‘:parent’

You don't have any slot named `:parent` nor `:padding-y`.
You're confusing the slots's names with the slots's initargs.

You want:

    (slot-boundp box 'parent)

rather than

    (slot-boundp box :parent)

BTW, while I don't like `defclass` (preferring `cl-defstruct`), one of
its neat features is the ability to override `slot-unbound`, which
should let you implement in a kind of "transparent" way the kind of
fallback mechanism that you're trying to implement with
`boxy--inherited`.

> I have two more questions about the patch. First, just to make sure,
> would you like to contribute those three diffs and did you sign the FSF
> Copyright paperwork?

;-)
Yes, I did.

>> +  (let ((inhibit-message t))     ;FIXME: Please report the message as an error.
>>      (setq indent-tabs-mode nil)
>>      (cursor-sensor-mode t)
>>      (toggle-truncate-lines t)))
>
> This doesn't even do what I want. I don't want 'Truncate long lines
> enabled' to be printed to the minibuffer when entering boxy mode, but it
> shows regardless. What exactly does the FIXME mean here and how do I
> stop that message?

The FIXME means that `toggle-truncate-lines` should not emit the message
at all in this case, just like `cursor-sensor-mode` doesn't emit
a message when called as above.
IOW, you should `M-x report-emacs-bug` and complain about it.


        Stefan

Re: [ELPA] New packages: boxy, boxy-headlines

[Tyler Grinn]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> That's what I've gathered to, so to get around that is why I'm using
>> face-spec-set. The behavior I want is this: update the face if a user
>> has not independently customized it already. If I simply changed the
>> defface, only new users would get the new theme. Existing users would
>> have to re-apply the standard definition or restart emacs.
>
> What you want is not specific to your theme, so it's best to
> fix it at its source rather than work around it with non-standard code.
> I suggest you `M-x report-emacs-bug` and request this behavior.
>

Will do.

>> (boxy--inherited boxy--padding-y :padding-y boxy--default-padding-y)
>>
>> But this results in
>> Warning: Unknown slot ‘:padding-y’
>> Warning: Unknown slot ‘:parent’
>
> You don't have any slot named `:parent` nor `:padding-y`.
> You're confusing the slots's names with the slots's initargs.
>
> You want:
>
>     (slot-boundp box 'parent)
>
> rather than
>
>     (slot-boundp box :parent)
>
> BTW, while I don't like `defclass` (preferring `cl-defstruct`), one of
> its neat features is the ability to override `slot-unbound`, which
> should let you implement in a kind of "transparent" way the kind of
> fallback mechanism that you're trying to implement with
> `boxy--inherited`.
>

That's an interesting revelation! I'll have to give that some thought
but I think it'll be useful for more than just inheritance.

>
>>> +  (let ((inhibit-message t))     ;FIXME: Please report the message as an error.
>>>      (setq indent-tabs-mode nil)
>>>      (cursor-sensor-mode t)
>>>      (toggle-truncate-lines t)))
>>
>> This doesn't even do what I want. I don't want 'Truncate long lines
>> enabled' to be printed to the minibuffer when entering boxy mode, but it
>> shows regardless. What exactly does the FIXME mean here and how do I
>> stop that message?
>
> The FIXME means that `toggle-truncate-lines` should not emit the message
> at all in this case, just like `cursor-sensor-mode` doesn't emit
> a message when called as above.
> IOW, you should `M-x report-emacs-bug` and complain about it.
>

Ok will do.

Thanks again for taking a look at this. I had some issues with the ELPA
deployment but I think I took care of them.

It looks like ELPA grabbed code from before I sent the first email. From
the beginning of the boxy-headlines repo I've been using 1.0.0 as the
version so maybe that explains it. There's a call to
org-real--add-headline which was renamed to
boxy-headlines--add-headline. I just pushed out version 1.0.1 with your
changes to both projects, so that should update boxy-headlines, right?

It looks like .elpaignore should only use relative paths. I will update
those accordingly so the test cases in boxy.el are not compiled.

The brief for boxy-headlines looks a little awkward in org format. If I
add the README.org and demo folder to .elpaignore it should fall back to
the "Commentary" and shrink the package size considerably.



Tyler

Re: [ELPA] New packages: boxy, boxy-headlines

[Tyler Grinn]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> That's what I've gathered to, so to get around that is why I'm using
>> face-spec-set. The behavior I want is this: update the face if a user
>> has not independently customized it already. If I simply changed the
>> defface, only new users would get the new theme. Existing users would
>> have to re-apply the standard definition or restart emacs.
>
> What you want is not specific to your theme, so it's best to
> fix it at its source rather than work around it with non-standard code.
> I suggest you `M-x report-emacs-bug` and request this behavior.
>
>         Stefan

The behavior of defface was changed in Emacs 28 to do what I
described. What do I do now?  Should I still support 26 and 27?

Re: [ELPA] New packages: boxy, boxy-headlines

[Stefan Monnier]

> The behavior of defface was changed in Emacs 28 to do what I
> described. What do I do now?  Should I still support 26 and 27?

AFAIU the difference is only visible when a running Emacs session loads
the `boxy.el` file a second time.  This may be common for the one who's
hacking on `boxy.el`, but it's probably *very* rare for every one else,
so I wouldn't worry about it.
Especially since that same behavior affects all other packages.


        Stefan

Re: [ELPA] New packages: boxy, boxy-headlines

[Stefan Monnier]

> It looks like ELPA grabbed code from before I sent the first email.

The release tarball is built from the commit in which you modify the
`Version:` header.  It's the act of changing this header which triggers
the release of a new version.


        Stefan

Re: [ELPA] New packages: boxy, boxy-headlines

[Tyler Grinn]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> The behavior of defface was changed in Emacs 28 to do what I
>> described. What do I do now?  Should I still support 26 and 27?
>
> AFAIU the difference is only visible when a running Emacs session loads
> the `boxy.el` file a second time.  This may be common for the one who's
> hacking on `boxy.el`, but it's probably *very* rare for every one else,
> so I wouldn't worry about it.
> Especially since that same behavior affects all other packages.
>
>
>         Stefan
>

I agree generally, but I know a lot of users tend to leave Emacs running
for weeks, and the problem I had was that the first color theme I
designed was inaccessible for dark themes, so I thought it was important
to send that update right away.  I think the theming is pretty
accessible and stable now, so I will remove the face-spec-set calls in
the next update.


-- 

Best,

Tyler

Re:Re: [ELPA] New packages: boxy, boxy-headlines

[tumashu]

>BTW, while I don't like `defclass` (preferring `cl-defstruct`), one of

Wow, why, I would like know the reason :-),  I do not use both of them :-).

Re: [ELPA] New packages: boxy, boxy-headlines

[Stefan Monnier]

tumashu [2021-10-13 16:48:49] wrote:
>>BTW, while I don't like `defclass` (preferring `cl-defstruct`), one of
> Wow, why, I would like know the reason :-),  I do not use both of them :-).

Mostly because its field access is inherently more costly (requires
a hash-table lookup, basically), because of its unrestricted support for
multiple inheritance.


        Stefan