From: Philip Kaludercic <philipk@posteo.net>
To: Protesilaos Stavrou <info@protesilaos.com>
Cc: emacs-devel@gnu.org
Subject: Re: [ELPA] Add new 'show-font' package?
Date: Thu, 05 Sep 2024 10:18:50 +0000 [thread overview]
Message-ID: <87jzfqjtdh.fsf@posteo.net> (raw)
In-Reply-To: <87msknu635.fsf@protesilaos.com> (Protesilaos Stavrou's message of "Wed, 04 Sep 2024 12:21:50 +0300")
[-- Attachment #1: Type: text/plain, Size: 1754 bytes --]
Protesilaos Stavrou <info@protesilaos.com> writes:
> Hello everyone,
>
> I am working on a new package called 'show-font'. It is a major mode for
> ".ttf" and ".otf" files (font files). It previews the current font, if
> available on the system, else it produces a helpful message that the
> font is not installed and thus cannot be previewed.
>
> These are early days. My immediate goals include the following:
>
> - Make it work on other operating systems beside GNU/Llinux.
> - Let users install a font file (OS-dependent).
> - Preview a font that is not installed on the system.
>
> I attach the patch I want to commit to elpa.git. Please let me know if
> you have any thoughts about this.
>
> All the best,
> Protesilaos (or simply "Prot")
>
> --
> Protesilaos Stavrou
> https://protesilaos.com
>
> From 6d0b5b4555c3ad80840c7260a276904c48fdf84e Mon Sep 17 00:00:00 2001
> Message-ID: <6d0b5b4555c3ad80840c7260a276904c48fdf84e.1725441140.git.info@protesilaos.com>
> From: Protesilaos Stavrou <info@protesilaos.com>
> Date: Wed, 4 Sep 2024 12:11:37 +0300
> Subject: [PATCH] * elpa-packages (show-font): Add new package
>
> ---
> elpa-packages | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/elpa-packages b/elpa-packages
> index 6bac9fb..7434a5d 100644
> --- a/elpa-packages
> +++ b/elpa-packages
> @@ -680,6 +680,8 @@
> (shell-command+ :url "https://git.sr.ht/~pkal/shell-command-plus")
> (shell-quasiquote :url nil)
> (shen-mode :url nil)
> + (show-font :url "https://github.com/protesilaos/show-font"
> + :readme "README.md")
> (sisu-mode :url nil)
> (site-lisp :url "https://git.sr.ht/~pkal/site-lisp")
> (sketch-mode :url "https://github.com/dalanicolai/sketch-mode")
A few comments on the code here:
[-- Attachment #2: Type: text/plain, Size: 5219 bytes --]
diff --git a/show-font.el b/show-font.el
index 4ecd184..519d838 100644
--- a/show-font.el
+++ b/show-font.el
@@ -6,7 +6,7 @@
;; Maintainer: Protesilaos Stavrou <info@protesilaos.com>
;; URL: https://github.com/protesilaos/show-font
;; Version: 0.0.0
-;; Package-Requires: ((emacs "28.1"))
+;; Package-Requires: ((emacs "28.1")) ;any reason for 28.1?
;; Keywords: convenience, writing, font
;; This file is NOT part of GNU Emacs.
@@ -49,7 +49,7 @@
(const :tag "Grumpy wizards make toxic brew for the evil queen and jack" wizards)
(const :tag "A quick movement of the enemy will jeopardize six gunboats" gunboats)
(const :tag "Prot may find zesty owls join quiet vixens as the night beckons" prot)
- string)
+ (string :tag "A custom pangram"))
:group 'show-font)
(defcustom show-font-character-sample
@@ -101,8 +101,7 @@ x×X .,·°;:¡!¿?`'‘’ ÄAÃÀ TODO
"Regular expression to match font file extensions.")
(defconst show-font-latin-alphabet
- '("a" "b" "c" "d" "e" "f" "g" "h" "i" "j" "k" "l" "m"
- "n" "o" "p" "q" "r" "s" "t" "u" "v" "w" "x" "y" "z")
+ (eval-when-compile (mapcar #'string (number-sequence ?a ?z)))
"The latin alphabet as a list of strings.")
(defun show-font-pangram-p (string &optional characters)
@@ -123,6 +122,9 @@ that all of them occur at least once in STRING."
;; mechanics of file handling yet. The idea of what I want is to get
;; an empty buffer. Then I add contents there without making it
;; appear modified.
+
+;; This constitutes a significant contribution, does it not?
+
(defun show-font-handler (operation &rest args)
"Handle the given I/O `file-name-handler-alist' OPERATION with ARGS.
Determine how to render the font file contents in a buffer."
@@ -154,8 +156,10 @@ matched against the output of the `fc-scan' executable."
(unless (executable-find "fc-scan")
(error "Cannot find `fc-scan' executable; will not render font"))
(when-let ((f (or file buffer-file-name))
- (_ (string-match-p show-font-extensions-regexp f))
- (output (shell-command-to-string (format "fc-scan -f \"%%{%s}\" %s" attribute f))))
+ ((string-match-p show-font-extensions-regexp f))
+ (output (shell-command-to-string (format "fc-scan -f \"%%{%s}\" %s"
+ (shell-quote-argument attribute)
+ (shell-quote-argument f)))))
(if (string-match-p "," output)
(car (split-string output ","))
output)))
@@ -163,7 +167,7 @@ matched against the output of the `fc-scan' executable."
(defun show-font--get-installed-fonts (&optional attribute)
"Get list of font families available on the system.
With optional ATTRIBUTE use it instead of \"family\"."
- (unless (executable-find "fc-list")
+ (unless (executable-find "fc-list") ;perhaps add a user option for the command name?
(error "Cannot find `fc-list' executable; will not find installed fonts"))
(process-lines
"fc-list"
@@ -189,6 +193,9 @@ With optional ATTRIBUTE use it instead of \"family\"."
"Prot may find zesty owls join quiet vixens as the night beckons")
(t
"No string or acceptable symbol value for `show-font-pangram', but this will do...")))
+;; Instead of duplicating the strings here and in the documentation,
+;; why not add a defconst with the panagrams that you use to simplify
+;; this function and generate the type for `show-font-pangram'?
(defun show-font--prepare-text ()
"Prepare pangram text at varying font heights."
@@ -219,10 +226,11 @@ With optional ATTRIBUTE use it instead of \"family\"."
(propertize "Rendered with parent family:" 'face (list 'show-font-regular :family family)) "\n"
(propertize family 'face (list 'show-font-subtitle :family family)) "\n"
(propertize (make-string (length family) ?=) 'face (list 'show-font-subtitle :family family)) "\n\n"
+ ;; Why not use `make-separator-line' here?
(mapconcat #'identity (nreverse list-of-lines) "\n") "\n"
(mapconcat #'identity (nreverse list-of-blocks) "\n") "\n"))))))
-(defmacro show-font-with-unmodified-buffer (&rest body)
+(defmacro show-font-with-unmodified-buffer (&rest body) ;isn't this `with-silent-modifications'?
"Run BODY while not making the buffer appear modified."
`(progn
,@body
@@ -241,7 +249,7 @@ buffer."
;;;###autoload
(define-derived-mode show-font-mode special-mode "Show Font"
"Major mode to preview a font file's character set."
- (set-buffer-multibyte t)
+ (set-buffer-multibyte t) ;is this safe?
(setq-local truncate-lines t
buffer-undo-list t
auto-save-default nil
@@ -252,6 +260,10 @@ buffer."
;; FIXME 2024-08-25: Do we want to autoload this or does it belong
;; somewhere else? It seems wrong like this.
+;; This seems fine to me. What I found more peculiar is the autoload
+;; in front of show-font-extensions-regexp, even if I understand the
+;; technical reasons for it.
+
;;;###autoload
(add-to-list 'file-name-handler-alist (cons show-font-extensions-regexp #'show-font-handler))
[-- Attachment #3: Type: text/plain, Size: 112 bytes --]
Some general comments: It would be nice to have a command to select a
font.
--
Philip Kaludercic on siskin
next prev parent reply other threads:[~2024-09-05 10:18 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-09-04 9:21 [ELPA] Add new 'show-font' package? Protesilaos Stavrou
2024-09-05 10:18 ` Philip Kaludercic [this message]
2024-09-05 10:24 ` Eli Zaretskii
2024-09-06 5:35 ` Protesilaos Stavrou
2024-09-06 6:12 ` Eli Zaretskii
2024-09-06 6:23 ` Protesilaos Stavrou
2024-09-06 7:16 ` Eli Zaretskii
2024-09-06 7:23 ` Protesilaos Stavrou
2024-09-06 10:47 ` Eli Zaretskii
2024-09-06 10:59 ` Protesilaos Stavrou
2024-09-06 11:40 ` Eli Zaretskii
2024-09-06 13:40 ` Protesilaos Stavrou
2024-09-06 13:56 ` Eli Zaretskii
2024-09-06 5:43 ` Protesilaos Stavrou
2024-09-06 6:15 ` Eli Zaretskii
2024-09-06 6:29 ` Protesilaos Stavrou
2024-09-06 7:17 ` Eli Zaretskii
2024-09-06 6:33 ` Philip Kaludercic
2024-09-06 6:45 ` Protesilaos Stavrou
2024-09-06 7:20 ` Eli Zaretskii
2024-09-06 7:31 ` Philip Kaludercic
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87jzfqjtdh.fsf@posteo.net \
--to=philipk@posteo.net \
--cc=emacs-devel@gnu.org \
--cc=info@protesilaos.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.