Adding new package hideshowvis to nongnu elpa

Adding new package hideshowvis to nongnu elpa

[Jan Rehders]

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

Hello,

I’d like to offer my package hideshowvis to be included in nongnu elpa. It has a few users since many years and since Melpa removed Emacs wiki packages years ago there has not been a trivial way to install it

Kind regards,
Jan

[-- Attachment #2: 0001-elpa-packages-hideshowvis-New-package.patch --]
[-- Type: application/octet-stream, Size: 892 bytes --]

From 9d2e52bf2e07e8b6a6f1049b4a74d34efdb58a4b Mon Sep 17 00:00:00 2001
From: Jan Rehders <nospam@sheijk.net>
Date: Tue, 28 May 2024 16:18:56 +0200
Subject: [PATCH] elpa-packages (hideshowvis): New package

---
 elpa-packages | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/elpa-packages b/elpa-packages
index 82dc12718b..6bb7524f86 100644
--- a/elpa-packages
+++ b/elpa-packages
@@ -371,6 +371,12 @@
 		  "helm-pkg.el" "helm-regexp.el" "helm-ring.el" "helm-semantic.el" "helm-shell.el"
 		  "helm-sys.el" "helm-tags.el" "helm-types.el" "helm-utils.el" "helm-x-files.el"))
 
+ (hideshowvis
+  :url "https://github.com/sheijk/hideshowvis/"
+  :branch "main"
+  :ignored-files ("screenshot.png")
+  :readme "README.md")
+
  (highlight-parentheses	:url "https://git.sr.ht/~tsdh/highlight-parentheses.el"
   :branch "main"
   :readme "README.md"
-- 
2.44.0

Re: Adding new package hideshowvis to nongnu elpa

[Philip Kaludercic]

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

Jan Rehders <jan@sheijk.net> writes:

> Hello,
>
> I’d like to offer my package hideshowvis to be included in nongnu
> elpa. 

Are you sure you don't want the package to be added to GNU ELPA?

>       It has a few users since many years and since Melpa removed
> Emacs wiki packages years ago there has not been a trivial way to
> install it

Here are a few comments and suggestions from looking at the code:


[-- Attachment #2: Type: text/plain, Size: 12437 bytes --]

diff --git a/hideshowvis.el b/hideshowvis.el
index 745afe8..fa4f863 100644
--- a/hideshowvis.el
+++ b/hideshowvis.el
@@ -1,12 +1,12 @@
 ;;; hideshowvis.el --- Fringe markers for regions foldable by hideshow.el -*- lexical-binding: t; -*-
-;;
+
 ;; Copyright 2008-2018 Jan Rehders
-;;
+
 ;; Author: Jan Rehders <jan@sheijk.net>
 ;; URL: https://github.com/sheijk/hideshowvis
 ;; Version: 0.8
-;; Package-Requires: ((emacs "24"))
-;;
+;; Package-Requires: ((emacs "27.1"))
+
 ;; Contributions and bug fixes by Bryan Waite, Michael Heerdegen, John Yates,
 ;; Matthew Fidler, and Eyal Soha.
 ;;
@@ -24,18 +24,19 @@
 ;; along with GNU Emacs; see the file COPYING.  If not, write to
 ;; the Free Software Foundation, Inc., 59 Temple Place - Suite 330,
 ;; Boston, MA 02111-1307, USA.
-;;
-;;
+
 ;;; Commentary:
+
+;; This minor mode will add little +/- displays to foldable regions in
+;; the buffer and to folded regions.  It is indented to be used in
+;; conjunction with hideshow.el which is a part of GNU Emacs since
+;; version 20.
 ;;
-;; This minor mode will add little +/- displays to foldable regions in the
-;; buffer and to folded regions.  It is indented to be used in conjunction with
-;; hideshow.el which is a part of GNU Emacs since version 20.
-;;
-;; Currently it works for me but is not tested heavily.  Please report any bugs
-;; to the above email address
-;;
+;; Currently it works for me but is not tested heavily.  Please report
+;; any bugs to the above email address
+
 ;;; Installation:
+
 ;; Add the following to your .emacs:
 ;;
 ;; (autoload 'hideshowvis-enable "hideshowvis" "Highlight foldable regions")
@@ -63,16 +64,16 @@
 ;;
 ;; It is not enabled by default because it might interfere with custom
 ;; hs-set-up-overlay functions
-;;
+
 ;;; TODO
-;;
+
 ;; - global-hideshowvis-minor-mode
 ;; - defcustom for hideshowvis-max-file-size needs to offer setting to nil
 ;; - add fringe icons lazily, only for visible region (check lazy font-lock to
 ;;   see if it can help with this)
-;;
+
 ;;; Changelog
-;;
+
 ;; v0.8, 2024-05-28
 ;; - Fixed interaction with auto-revert-mode and indent-region
 ;; - Fixed performance issue in some cases due to leaking overlays
@@ -99,27 +100,29 @@
 ;; v0.2, 2009-08-09
 ;; - The '-' symbol in fringe is clickable.
 ;; - Don't show '-' in fringe if the foldable region ends on the same line.
-;;
+
 ;;; Code:
 
 (require 'hideshow)
 
+(defgroup hideshowvis ()
+  "Fringe markers for regions foldable by hideshow"
+  :group 'hideshow)
+
 (define-fringe-bitmap 'hideshowvis-hideable-marker [0 0 0 126 126 0 0 0])
 
 (defconst hideshowvis-version "v0.7" "Version of hideshowvis minor mode.")
 
 (defface hideshowvis-hidable-face
   '((t (:foreground "#ccc" :box t)))
-  "Face to highlight foldable regions."
-  :group 'hideshow)
+  "Face to highlight foldable regions.")
 
 (defcustom hideshowvis-ignore-same-line t
   "No + for single line regions.
 Do not display foldable regions in the fringe if the matching closing
 parenthesis is on the same line.  Set this to nil if enabling the minor mode is
 slow on your machine"
-  :group 'hideshow
-  :type 'bool)
+  :type 'boolean)
 
 (defcustom hideshowvis-max-file-size (* 1024 100)
   "No highlighting in files larger than this number of bytes.
@@ -127,8 +130,7 @@ slow on your machine"
 ‘hideshowvis-enable’ will not enable hideshowvis-mode if file is larger than
 this value (in bytes).  The minor mode can still be forced to be enabled using
 `(hideshowvis-mode 1)'.  Set this variable to nil to disable restriction."
-  :group 'hideshow
-  :type 'integer)
+  :type 'natnum)
 
 (defun hideshowvis-highlight-hs-regions-in-fringe (&optional start end _old-text-length)
   "Will update the fringe indicators for all foldable regions in the buffer.
@@ -146,11 +148,9 @@ functions used with `after-change-functions'."
         (remove-overlays (point-min) (point-max) 'hideshowvis-hs t)
         (while (search-forward-regexp hs-block-start-regexp nil t)
           (when (if hideshowvis-ignore-same-line
-                    (let (begin-line)
-                      (setq begin-line
-                            (save-excursion
-                              (goto-char (match-beginning 0))
-                              (line-number-at-pos (point))))
+                    (let ((begin-line (save-excursion
+					(goto-char (match-beginning 0))
+					(line-number-at-pos (point)))))
                       (save-excursion
                         (goto-char (match-beginning 0))
                         (ignore-errors
@@ -158,16 +158,14 @@ functions used with `after-change-functions'."
                             (funcall hs-forward-sexp-func 1)
                             (> (line-number-at-pos (point)) begin-line)))))
                   t)
-            (let* ((ovl (make-overlay (match-beginning 0) (match-end 0)))
-                   (marker-string "*hideshowvis*"))
-              (put-text-property 0
-                                 (length marker-string)
-                                 'display
-                                 (list 'left-fringe
-                                       'hideshowvis-hideable-marker
-                                       'hideshowvis-hidable-face)
-                                 marker-string)
-              (overlay-put ovl 'before-string marker-string)
+            (let ((ovl (make-overlay (match-beginning 0) (match-end 0))))
+              (overlay-put ovl 'before-string
+			   (propertize
+			    "*hideshowvis*"
+			    'display
+                            (list 'left-fringe
+                                  'hideshowvis-hideable-marker
+                                  'hideshowvis-hidable-face)))
               (overlay-put ovl 'hideshowvis-hs t))))))))
 
 ;;;###autoload
@@ -188,34 +186,32 @@ functions used with `after-change-functions'."
     (hs-hide-block)
     (beginning-of-line)))
 
-(defvar hideshowvis-mode-map
+(define-obsolete-variable-alias 'hideshowvis-mode-map
+  'hideshowvis-minor-mode-map
+  "0.9")
+
+(defvar hideshowvis-minor-mode-map
   (let ((hideshowvis-mode-map (make-sparse-keymap)))
     (define-key hideshowvis-mode-map [left-fringe mouse-1]
       'hideshowvis-click-fringe)
     hideshowvis-mode-map)
-  "Keymap for hideshowvis mode.")
+  "Keymap for `hideshowvis-minor-mode'.")
 
 ;;;###autoload
-(define-minor-mode hideshowvis-minor-mode ()
-  :doc "Will indicate regions foldable with hideshow in the fringe."
-  :init-value nil
-  :require 'hideshow
-  :group 'hideshow
-  :keymap hideshowvis-mode-map
-  (condition-case nil
+(define-minor-mode hideshowvis-minor-mode
+  "Will indicate regions foldable with hideshow in the fringe."
+  :global nil
+  (condition-case err
       (if hideshowvis-minor-mode
           (progn
             (hs-minor-mode 1)
             (hideshowvis-highlight-hs-regions-in-fringe (point-min) (point-max) 0)
-            (add-to-list 'after-change-functions
-                         'hideshowvis-highlight-hs-regions-in-fringe))
+            (add-hook 'after-change-functions
+                      #'hideshowvis-highlight-hs-regions-in-fringe))
         (remove-overlays (point-min) (point-max) 'hideshowvis-hs t)
-        (setq after-change-functions
-              (remove 'hideshowvis-highlight-hs-regions-in-fringe
-                      after-change-functions)))
-    (error
-     (message "Failed to toggle hideshowvis-minor-mode")
-     )))
+	(remove-hook 'after-change-functions
+		     #'hideshowvis-highlight-hs-regions-in-fringe))
+    (error (message "Failed to toggle `hideshowvis-minor-mode': %S" err))))
 
 ;;;###autoload
 (defun hideshowvis-enable ()
@@ -229,37 +225,35 @@ functions used with `after-change-functions'."
 
 (defcustom hideshowvis-hidden-fringe-face 'hideshowvis-hidden-fringe-face
   "*Specify face used to highlight the fringe on hidden regions."
-  :type 'face
-  :group 'hideshow)
+  :type 'face)
 
-(defface hideshowvis-hidden-fringe-face
+(defface hideshowvis-hidden-fringe-face	;perhaps alias `mode-line-highlight'
   '((t (:foreground "#888" :box (:line-width 2 :color "grey75" :style released-button))))
-  "Face used to highlight the fringe on folded regions."
-  :group 'hideshow)
+  "Face used to highlight the fringe on folded regions.")
 
 (defcustom hideshowvis-hidden-region-face 'hideshowvis-hidden-region-face
   "*Specify the face to to use for the hidden region indicator."
-  :type 'face
-  :group 'hideshow)
+  :type 'face)
 
 (defface hideshowvis-hidden-region-face
   '((t (:background "#ff8" :box t)))
-  "Face to hightlight the ... area of hidden regions."
-  :group 'hideshow)
+  "Face to hightlight the ... area of hidden regions.")
 
 (defun hideshowvis-display-code-line-counts (ov)
   "Extend overlay OV to show number of lines hidden for `hideshowvis-symbols'."
   (when (eq 'code (overlay-get ov 'hs))
-    (let* ((marker-string "*fringe-dummy*")
-           (marker-length (length marker-string))
-           (display-string (format "%d lines" (count-lines (overlay-start ov) (overlay-end ov)))))
-      (overlay-put ov 'help-echo "Hidden text. C-c,= to show")
-      (put-text-property 0 marker-length 'display
-                         (list 'left-fringe 'hideshowvis-hidden-marker 'hideshowvis-hidden-fringe-face)
-                         marker-string)
-      (overlay-put ov 'before-string marker-string)
-      (put-text-property 0 (length display-string) 'face 'hideshowvis-hidden-region-face display-string)
-      (overlay-put ov 'after-string display-string))))
+    (overlay-put ov 'help-echo "Hidden text. C-c,= to show")
+    (overlay-put ov 'before-string
+		 (propertize
+		  "*fringe-dummy*"
+		  'display
+		  '(left-fringe
+		    hideshowvis-hidden-marker
+		    hideshowvis-hidden-fringe-face)))
+    (overlay-put ov 'after-string
+		 (propertize
+		  (format "%d lines" (count-lines (overlay-start ov) (overlay-end ov)))
+		  'face 'hideshowvis-hidden-region-face))))
 
 ;;;###autoload
 (defun hideshowvis-symbols ()
@@ -271,12 +265,12 @@ indicating the number of hidden lines at the end of the line for hidden regions.
 This will change the value of `hs-set-up-overlay' so it will
 overwrite anything you've set there."
   (interactive)
-  (setq hs-set-up-overlay 'hideshowvis-display-code-line-counts)
+  (setq hs-set-up-overlay #'hideshowvis-display-code-line-counts)
   ;; These won't get removed, again. Revert hooks are global and making them
   ;; buffer local might be risky. Instead checking whether showing symbols is
   ;; turned on in the hook functions
-  (add-hook 'before-revert-hook 'hideshowvis-remove-overlays)
-  (add-hook 'after-revert-hook 'hideshowvis-update-all-overlays)
+  (add-hook 'before-revert-hook #'hideshowvis-remove-overlays)
+  (add-hook 'after-revert-hook #'hideshowvis-update-all-overlays)
   (hideshowvis-update-all-overlays))
 
 ;;;###autoload
@@ -284,21 +278,21 @@ overwrite anything you've set there."
   "Disable enhanced highlighting of hidden regions."
   (interactive)
   (hideshowvis-remove-overlays)
-  (setq hs-set-up-overlay 'ignore))
+  (setq hs-set-up-overlay #'ignore))
 
 (defun hideshowvis-remove-overlays ()
   "Will remove all overlays added after calling `hideshowvis-symbols'."
-  (when (equal hs-set-up-overlay 'hideshowvis-display-code-line-counts)
+  (when (eq hs-set-up-overlay #'hideshowvis-display-code-line-counts)
     (dolist (ov (overlays-in (point-min) (point-max)))
-      (unless (null (overlay-get ov 'hs))
+      (when (overlay-get ov 'hs)	;why are you returning a `hs' here?
         (overlay-put ov 'after-string nil)))))
 
 (defun hideshowvis-update-all-overlays ()
   "Will update all overlays added after calling `hideshowvis-symbols'."
-  (when (equal hs-set-up-overlay 'hideshowvis-display-code-line-counts)
+  (when (eq hs-set-up-overlay #'hideshowvis-display-code-line-counts)
     (dolist (ov (overlays-in (point-min) (point-max)))
-      (unless (null (overlay-get ov 'hs))
-        (if (equal (overlay-start ov) (overlay-end ov))
+      (when (overlay-get ov 'hs)
+        (if (= (overlay-start ov) (overlay-end ov))
             (delete-overlay ov)
           (hideshowvis-display-code-line-counts ov))))))
 

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


> Kind regards,
> Jan
>
> From 9d2e52bf2e07e8b6a6f1049b4a74d34efdb58a4b Mon Sep 17 00:00:00 2001
> From: Jan Rehders <nospam@sheijk.net>
> Date: Tue, 28 May 2024 16:18:56 +0200
> Subject: [PATCH] elpa-packages (hideshowvis): New package
>
> ---
>  elpa-packages | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/elpa-packages b/elpa-packages
> index 82dc12718b..6bb7524f86 100644
> --- a/elpa-packages
> +++ b/elpa-packages
> @@ -371,6 +371,12 @@
>  		  "helm-pkg.el" "helm-regexp.el" "helm-ring.el" "helm-semantic.el" "helm-shell.el"
>  		  "helm-sys.el" "helm-tags.el" "helm-types.el" "helm-utils.el" "helm-x-files.el"))
>  
> + (hideshowvis
> +  :url "https://github.com/sheijk/hideshowvis/"
> +  :branch "main"
> +  :ignored-files ("screenshot.png")

You can track files to ignore in a .elpaignore file as well.  That might
be better, in case you decide to replace, rename the file or add more.
I can change that for you, you just have to add the file to the
repository.

> +  :readme "README.md")
> +
>   (highlight-parentheses	:url "https://git.sr.ht/~tsdh/highlight-parentheses.el"
>    :branch "main"
>    :readme "README.md"

-- 
	Philip Kaludercic on peregrine

Re: Adding new package hideshowvis to nongnu elpa

[Jan Rehders]

I’ve applied your improvements, thank you!

> Are you sure you don't want the package to be added to GNU ELPA?

If that requires copyright assignment then I’m happy to do that but maybe at a later time. I’m not sure I have the time+energy to do that and check if any contributors submitted more than a few lines right now

> (defun hideshowvis-remove-overlays ()
>   "Will remove all overlays added after calling `hideshowvis-symbols'."
> -  (when (equal hs-set-up-overlay 'hideshowvis-display-code-line-counts)
> +  (when (eq hs-set-up-overlay #'hideshowvis-display-code-line-counts)
>     (dolist (ov (overlays-in (point-min) (point-max)))
> -      (unless (null (overlay-get ov 'hs))
> +      (when (overlay-get ov 'hs)	;why are you returning a `hs' here?
>         (overlay-put ov 'after-string nil)))))

Not sure I understand the question - I don’t intend to return anything. I’m using the ‘hs property added by hideshow.el to identify which overlays need to be changed to not break other highlighting packages.

> You can track files to ignore in a .elpaignore file as well.  That might
> be better, in case you decide to replace, rename the file or add more.
> I can change that for you, you just have to add the file to the
> repository.

This seems like a good idea, added .elpaignore to the repo

Re: Adding new package hideshowvis to nongnu elpa

[Philip Kaludercic]

Jan Rehders <jan@sheijk.net> writes:

> I’ve applied your improvements, thank you!
>
>> Are you sure you don't want the package to be added to GNU ELPA?
>
> If that requires copyright assignment then I’m happy to do that but
> maybe at a later time. I’m not sure I have the time+energy to do that
> and check if any contributors submitted more than a few lines right
> now

No problem, I just wanted to make sure that you were consciously
deciding to submit the package to NonGNU.

>> (defun hideshowvis-remove-overlays ()
>>   "Will remove all overlays added after calling `hideshowvis-symbols'."
>> -  (when (equal hs-set-up-overlay 'hideshowvis-display-code-line-counts)
>> +  (when (eq hs-set-up-overlay #'hideshowvis-display-code-line-counts)
>>     (dolist (ov (overlays-in (point-min) (point-max)))
>> -      (unless (null (overlay-get ov 'hs))
>> +      (when (overlay-get ov 'hs)	;why are you returning a `hs' here?
>>         (overlay-put ov 'after-string nil)))))
>
> Not sure I understand the question - I don’t intend to return
> anything. I’m using the ‘hs property added by hideshow.el to identify
> which overlays need to be changed to not break other highlighting
> packages.

Ugh, my bad I misread the line and thought to have read

        (dolist (ov (overlays-in (point-min) (point-max)) 'hs)
          ...)

Sorry about that.

>> You can track files to ignore in a .elpaignore file as well.  That might
>> be better, in case you decide to replace, rename the file or add more.
>> I can change that for you, you just have to add the file to the
>> repository.
>
> This seems like a good idea, added .elpaignore to the repo

1+

Is there anything else that we should discuss before adding the package
to the archive?

-- 
	Philip Kaludercic on peregrine

Re: Adding new package hideshowvis to nongnu elpa

[Jan Rehders]

> Is there anything else that we should discuss before adding the package
> to the archive?

From my side it’s ready to go. Thanks for adding it

Re: Adding new package hideshowvis to nongnu elpa

[Philip Kaludercic]

Jan Rehders <jan@sheijk.net> writes:

>> Is there anything else that we should discuss before adding the package
>> to the archive?
>
> From my side it’s ready to go. Thanks for adding it

Done, sorry for the delay.

-- 
	Philip Kaludercic on peregrine