~protesilaos/denote

This thread contains a patchset. You're looking at the original emails, but you may wish to use the patch review UI. Review patch
5 2

[PATCH] proposed change to improve timestamp readability in denote file names

Jean-Charles Bagneris <lists@bagneris.net>
Details
Message ID
<48ef34ff-ce25-4d3a-be65-dcea6425fda9@app.fastmail.com>
DKIM signature
missing
Download raw message
Hello Prot and all,

Please find attached a proposed patch to improve the readability of the timestamp/id 
in the denote file name for 'denote-dired-mode' and the like. 

The idea was to make things as simple and unobtrusive as possible, 
thus I simply treated the "T" separator in the timestamp as a delimiter.

The change is minimal but my (allegedly old) eyes appreciate it greatly.

This is my first patch here, and I hope that I followed correctly the instructions
in https://protesilaos.com/codelog/2022-04-09-simple-guide-git-patches-emacs/,
please tell me if I misunderstood anything.

As usual, feel free to ignore, discuss or improve this minor change.

Have a nice day!
-- 
JcB
Details
Message ID
<87fs5ess9x.fsf@protesilaos.com>
In-Reply-To
<48ef34ff-ce25-4d3a-be65-dcea6425fda9@app.fastmail.com> (view parent)
DKIM signature
missing
Download raw message
> From: "Jean-Charles Bagneris" <lists@bagneris.net>
> Date: Sat, 22 Jul 2023 11:29:46 +0000
>
> Hello Prot and all,

Hello Jean-Charles,

> Please find attached a proposed patch to improve the readability of
> the timestamp/id in the denote file name for 'denote-dired-mode' and
> the like.
>
> The idea was to make things as simple and unobtrusive as possible, 
> thus I simply treated the "T" separator in the timestamp as a delimiter.

Good idea!

> The change is minimal but my (allegedly old) eyes appreciate it greatly.

I suggest we introduce a new 'denote-faces-time-delimiter' face.  This
gives users the option to revert back to the previous design or use a
more intense style for that matter.

The face can be a copy of 'denote-faces-delimiter', albeit with the new
:package-version.

Personally, I find the current delimiter colour too subtle for the "T".
Perhaps because I am used to its previous style.  I think using the
'shadow' face follows the spirit of your change while using a subtle
colour that is (hopefully) consistent with the underlying theme.

What do you think?

> This is my first patch here, and I hope that I followed correctly the instructions
> in https://protesilaos.com/codelog/2022-04-09-simple-guide-git-patches-emacs/,
> please tell me if I misunderstood anything.

It's all good.  Thank you!

All the best,
Prot

-- 
Protesilaos Stavrou
https://protesilaos.com
Jean-Charles Bagneris <lists@bagneris.net>
Details
Message ID
<3a7332cf-8044-4c04-b9e4-0942edcd70f0@app.fastmail.com>
In-Reply-To
<87fs5ess9x.fsf@protesilaos.com> (view parent)
DKIM signature
missing
Download raw message
On Sun, Jul 23, 2023, at 12:53, Protesilaos Stavrou wrote:
>
> Hello Jean-Charles,
>
> [...]
>
> I suggest we introduce a new 'denote-faces-time-delimiter' face.  This
> gives users the option to revert back to the previous design or use a
> more intense style for that matter.
>
> The face can be a copy of 'denote-faces-delimiter', albeit with the new
> :package-version.

Excellent, the possibility to revert back easily to the previous look and feel
would obviously be nice to some users.

> Personally, I find the current delimiter colour too subtle for the "T".
> Perhaps because I am used to its previous style.  I think using the
> 'shadow' face follows the spirit of your change while using a subtle
> colour that is (hopefully) consistent with the underlying theme.
>
> What do you think?

I actually felt the same (colour too subtle) but chose to introduce the change
with a minimal patch before investigating further. Using the 'shadow' face 
seems to be a good tradeoff, no problem for me.

Thank you for the feedback, and congrats for 2.0!
-- 
JcB
Details
Message ID
<87sf975p5n.fsf@protesilaos.com>
In-Reply-To
<3a7332cf-8044-4c04-b9e4-0942edcd70f0@app.fastmail.com> (view parent)
DKIM signature
missing
Download raw message
Patch: +10 -3
Hello again Jean-Charles and sorry for being slow to respond!

> From: "Jean-Charles Bagneris" <lists@bagneris.net>
> Date: Sun, 23 Jul 2023 13:49:33 +0000

> [... 17 lines elided]

>> Personally, I find the current delimiter colour too subtle for the "T".
>> Perhaps because I am used to its previous style.  I think using the
>> 'shadow' face follows the spirit of your change while using a subtle
>> colour that is (hopefully) consistent with the underlying theme.
>>
>> What do you think?
>
> I actually felt the same (colour too subtle) but chose to introduce the change
> with a minimal patch before investigating further. Using the 'shadow' face 
> seems to be a good tradeoff, no problem for me.
>
> Thank you for the feedback, and congrats for 2.0!

I have tweaked your patch based on this discussion.  I want to install
the following changes on your behalf (I will mention you in the manual
as well).  Please check them and let me know if everything is okay:


From a279e1cf1a11bb913d503d717daca060987668e5 Mon Sep 17 00:00:00 2001
Message-ID: <a279e1cf1a11bb913d503d717daca060987668e5.1690604032.git.info@protesilaos.com>
From: Jean-Charles Bagneris <lists@bagneris.net>
Date: Sat, 22 Jul 2023 11:29:46 +0000
Subject: [PATCH] Treat "T" of identifier as a delimiter in Denote file
 fontification

The idea was to make things as simple and unobtrusive as possible,
thus I simply treated the "T" separator in the timestamp as a
delimiter.  The effect is noticed in 'denote-dired-mode' and the
default backlinks buffer.

The introduction of a new face, 'denote-faces-time-delimiter', makes
it possible for users to revert to the previous style where the "T"
looks the same as the rest of the identifier.  Evaluate this:

(set-face-attribute 'denote-faces-time-delimiter nil :inherit 'denote-faces-date)
---
 denote.el | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/denote.el b/denote.el
index 322eade..804f9c5 100644
--- a/denote.el
+++ b/denote.el
@@ -2655,9 +2655,14 @@ (defface denote-faces-delimiter
  :group 'denote-faces
  :package-version '(denote . "0.1.0"))

(defface denote-faces-time-delimiter '((t :inherit shadow))
  "Face for the delimiter between date and time in Dired buffers."
  :group 'denote-faces
  :package-version '(denote . "2.1.0"))

;; For character classes, evaluate: (info "(elisp) Char Classes")
(defvar denote-faces--file-name-regexp
  (concat "\\(?1:[0-9]\\{8\\}\\)\\(?2:T[0-9]\\{6\\}\\)"
  (concat "\\(?1:[0-9]\\{8\\}\\)\\(?10:T\\)\\(?2:[0-9]\\{6\\}\\)"
          "\\(?:\\(?3:==\\)\\(?4:[[:alnum:][:nonascii:]=]*?\\)\\)?"
          "\\(?:\\(?5:--\\)\\(?6:[[:alnum:][:nonascii:]-]*?\\)\\)?"
          "\\(?:\\(?7:__\\)\\(?8:[[:alnum:][:nonascii:]_-]*?\\)\\)?"
@@ -2667,6 +2672,7 @@ (defvar denote-faces--file-name-regexp
(defconst denote-faces-file-name-keywords
  `((,(concat "[\t\s]+" denote-faces--file-name-regexp)
     (1 'denote-faces-date)
     (10 'denote-faces-time-delimiter nil t)
     (2 'denote-faces-time)
     (3 'denote-faces-delimiter nil t)
     (4 'denote-faces-signature nil t)
@@ -2678,9 +2684,10 @@ (defconst denote-faces-file-name-keywords
  "Keywords for fontification of file names.")

(defconst denote-faces-file-name-keywords-for-backlinks
  `((,(concat "^\\(?10:.*/\\)?" denote-faces--file-name-regexp)
     (10 'denote-faces-subdirectory nil t)
  `((,(concat "^\\(?11:.*/\\)?" denote-faces--file-name-regexp)
     (11 'denote-faces-subdirectory nil t)
     (1 'denote-faces-date)
     (10 'denote-faces-delimiter nil t)
     (2 'denote-faces-time)
     (3 'denote-faces-delimiter nil t)
     (4 'denote-faces-signature nil t)
-- 
2.41.0


All the best,
Prot

-- 
Protesilaos Stavrou
https://protesilaos.com
Jean-Charles Bagneris <lists@bagneris.net>
Details
Message ID
<1c96d90f-8e42-4e16-be21-bc892ec0749b@app.fastmail.com>
In-Reply-To
<87sf975p5n.fsf@protesilaos.com> (view parent)
DKIM signature
missing
Download raw message
Hello Prot,

On Sat, Jul 29, 2023, at 04:15, Protesilaos Stavrou wrote:
> Hello again Jean-Charles and sorry for being slow to respond!

No problem, I know that you were busy with the hut project recently,
and I hope that everything goes as smoothly as possible there.

> I have tweaked your patch based on this discussion.  I want to install
> the following changes on your behalf (I will mention you in the manual
> as well).  Please check them and let me know if everything is okay:

Thank you for this. I think there is a small typo or mistyping in the patch (see below),
otherwise everything looks fine to me.

> [5 lines elided]
> Subject: [PATCH] Treat "T" of identifier as a delimiter in Denote file
>  fontification
>
> [8 lines elided]
>
> (set-face-attribute 'denote-faces-time-delimiter nil :inherit 
> 'denote-faces-date)
> ---
>  denote.el | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/denote.el b/denote.el
> index 322eade..804f9c5 100644
> --- a/denote.el
> +++ b/denote.el
> @@ -2655,9 +2655,14 @@ (defface denote-faces-delimiter
>    :group 'denote-faces
>    :package-version '(denote . "0.1.0"))
> 
> +(defface denote-faces-time-delimiter '((t :inherit shadow))
> +  "Face for the delimiter between date and time in Dired buffers."
> +  :group 'denote-faces
> +  :package-version '(denote . "2.1.0"))
> +
>  ;; For character classes, evaluate: (info "(elisp) Char Classes")
>  (defvar denote-faces--file-name-regexp
> -  (concat "\\(?1:[0-9]\\{8\\}\\)\\(?2:T[0-9]\\{6\\}\\)"
> +  (concat "\\(?1:[0-9]\\{8\\}\\)\\(?10:T\\)\\(?2:[0-9]\\{6\\}\\)"
>            "\\(?:\\(?3:==\\)\\(?4:[[:alnum:][:nonascii:]=]*?\\)\\)?"
>            "\\(?:\\(?5:--\\)\\(?6:[[:alnum:][:nonascii:]-]*?\\)\\)?"
>            "\\(?:\\(?7:__\\)\\(?8:[[:alnum:][:nonascii:]_-]*?\\)\\)?"
> @@ -2667,6 +2672,7 @@ (defvar denote-faces--file-name-regexp
>  (defconst denote-faces-file-name-keywords
>    `((,(concat "[\t\s]+" denote-faces--file-name-regexp)
>       (1 'denote-faces-date)
> +     (10 'denote-faces-time-delimiter nil t)
>       (2 'denote-faces-time)
>       (3 'denote-faces-delimiter nil t)
>       (4 'denote-faces-signature nil t)
> @@ -2678,9 +2684,10 @@ (defconst denote-faces-file-name-keywords
>    "Keywords for fontification of file names.")
> 
>  (defconst denote-faces-file-name-keywords-for-backlinks
> -  `((,(concat "^\\(?10:.*/\\)?" denote-faces--file-name-regexp)
> -     (10 'denote-faces-subdirectory nil t)
> +  `((,(concat "^\\(?11:.*/\\)?" denote-faces--file-name-regexp)
> +     (11 'denote-faces-subdirectory nil t)
>       (1 'denote-faces-date)
> +     (10 'denote-faces-delimiter nil t)

Shouldn't this line above use 'denote-faces-time-delimiter as well?

Have a nice day!
-- 
JcB
Details
Message ID
<87v8dzfjnn.fsf@protesilaos.com>
In-Reply-To
<1c96d90f-8e42-4e16-be21-bc892ec0749b@app.fastmail.com> (view parent)
DKIM signature
missing
Download raw message
> From: "Jean-Charles Bagneris" <lists@bagneris.net>
> Date: Sat, 29 Jul 2023 10:39:19 +0000
>
> Hello Prot,

Hello Jean-Charles,

> [... 10 lines elided]

> Thank you for this. I think there is a small typo or mistyping in the
> patch (see below), otherwise everything looks fine to me.

Thank you!  I fixed that and pushed the change on your behalf.  Also
updated the manual to mention your name.

All the best,
Prot

-- 
Protesilaos Stavrou
https://protesilaos.com
Reply to thread Export thread (mbox)