GNU bug report logs - #40759
26.3; Update Leuven-theme in Emacs dev trunk

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 40759 in the body.
You can then email your comments to 40759 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Fab Rice <posh18 <at> pirilampo.be>
To: bug-gnu-emacs <at> gnu.org
Cc: dgutov <at> yandex.ru
Subject: 26.3; Update Leuven-theme in Emacs dev trunk
Date: Wed, 22 Apr 2020 07:31:21 +0200

[Message part 1 (text/plain, inline)]

Dear Emacs maintainers,

I'm the author of Leuven color theme, currently in Emacs since 24.4.

But the version has not been updated for a while, while I've improved it
with time.

Could you update its code with the following patch?

(I've generated it with 'git format-patch', so that you could apply it
with 'git am'.)

Thanks in advance!

Best regards,
Fabrice Niessen

[0001-Update-leuven-theme.el.patch (text/x-diff, attachment)]

Message #8 received at 40759 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefan <at> marxist.se>
To: Fab Rice <posh18 <at> pirilampo.be>
Cc: 40759 <at> debbugs.gnu.org, dgutov <at> yandex.ru
Subject: Re: bug#40759: 26.3; Update Leuven-theme in Emacs dev trunk
Date: Wed, 22 Apr 2020 08:10:56 +0200

Just one nitpick:

> -;; along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.
> +;; along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.

I think https is preferred here.

Best regards,
Stefan Kangas

Message #11 received at 40759 <at> debbugs.gnu.org (full text, mbox):

From: Fab Rice <posh18 <at> pirilampo.be>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: 40759 <at> debbugs.gnu.org, dgutov <at> yandex.ru
Subject: Re: bug#40759: 26.3; Update Leuven-theme in Emacs dev trunk
Date: Wed, 22 Apr 2020 21:58:39 +0200

[Message part 1 (text/plain, inline)]

Dear Stefan,

Please find here an update patch file, then.  Thanks for your comments!

BTW, could this go to the "emacs-27" branch, please?

Best regards,
Fabrice

[0001-Update-leuven-theme.el.patch (text/x-diff, attachment)]

Message #14 received at 40759 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefan <at> marxist.se>
To: Fab Rice <posh18 <at> pirilampo.be>
Cc: 40759 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>, dgutov <at> yandex.ru
Subject: Re: bug#40759: 26.3; Update Leuven-theme in Emacs dev trunk
Date: Thu, 23 Apr 2020 20:47:37 +0200

Fab Rice <posh18 <at> pirilampo.be> writes:

> BTW, could this go to the "emacs-27" branch, please?

I think the emacs-27 branch is closed for anything that is not
urgent.  Eli, what do you think?

A few more nitpicks:

> +(defcustom leuven-scale-org-agenda-structure t
> +  "Scale Org agenda structure lines, like dates.
> +This can be nil for unscaled, t for using the theme default, or a scaling number."

The last line should be reformatted to less than 80 characters, see:
(info "(elisp) Documentation Tips")

> +  :group 'leuven)

You don't need ":group 'leuven" here since it defaults to the previous
defgroup.

Best regards,
Stefan Kangas

Message #19 received at 40759 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Serghei <egrep <at> protonmail.ch>
Cc: posh18 <at> pirilampo.be, 40759 <at> debbugs.gnu.org, stefan <at> marxist.se,
 dgutov <at> yandex.ru
Subject: Re: bug#40759: 26.3; Update Leuven-theme in Emacs dev trunk
Date: Fri, 24 Apr 2020 14:26:53 +0300

> Date: Fri, 24 Apr 2020 10:57:35 +0000
> From: Serghei <egrep <at> protonmail.ch>
> Cc: 40759 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>, dgutov <at> yandex.ru
> 
> I asked for this merge Leuven Theme in 22 March [1] so I'm more than
> interested in merging this before 27.1 release.
> 
> Eli, Stefan consider please to release Emacs 27.1 with fresh Leuven
> version.

What about the issues pointed out by Stefan in his message?  Will an
updated patch be posted any time soon?

Message #22 received at 40759 <at> debbugs.gnu.org (full text, mbox):

From: Fab Rice <posh18 <at> pirilampo.be>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 40759 <at> debbugs.gnu.org, stefan <at> marxist.se, Serghei <egrep <at> protonmail.ch>,
 dgutov <at> yandex.ru
Subject: Re: bug#40759: 26.3; Update Leuven-theme in Emacs dev trunk
Date: Fri, 24 Apr 2020 14:09:56 +0200

[Message part 1 (text/plain, inline)]

Hello everybody,

Le 2020-04-24 13:26, Eli Zaretskii a écrit :
>> Date: Fri, 24 Apr 2020 10:57:35 +0000
>> From: Serghei <egrep <at> protonmail.ch>
>> Cc: 40759 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>, 
>> dgutov <at> yandex.ru
>> 
>> I asked for this merge Leuven Theme in 22 March [1] so I'm more than
>> interested in merging this before 27.1 release.
>> 
>> Eli, Stefan consider please to release Emacs 27.1 with fresh Leuven
>> version.
> 
> What about the issues pointed out by Stefan in his message?  Will an
> updated patch be posted any time soon?

Here's an updated patch file, where I've fixed the issues pointed by 
Stefan.

PS- I don't know why, I do not get all the messages from this thread...

Best regards,
Fabrice

[0001-Update-Leuven-theme.el.patch (text/x-diff, attachment)]

Message #25 received at 40759 <at> debbugs.gnu.org (full text, mbox):

From: Serghei <egrep <at> protonmail.ch>
To: Stefan Kangas <stefan <at> marxist.se>, Fab Rice <posh18 <at> pirilampo.be>
Cc: 40759 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>, dgutov <at> yandex.ru
Subject: Re: bug#40759: 26.3; Update Leuven-theme in Emacs dev trunk
Date: Fri, 24 Apr 2020 10:57:35 +0000

Hello,

I asked for this merge Leuven Theme in 22 March [1] so I'm more than
interested in merging this before 27.1 release.

Eli, Stefan consider please to release Emacs 27.1 with fresh Leuven
version.

[1]: https://github.com/fniessen/emacs-leuven-theme/issues/69

> Fab Rice <posh18 <at> pirilampo.be> writes:
> 
>> BTW, could this go to the "emacs-27" branch, please?
> 
> I think the emacs-27 branch is closed for anything that is not
> urgent.  Eli, what do you think?
> 
> A few more nitpicks:
> 
>> +(defcustom leuven-scale-org-agenda-structure t
>> +  "Scale Org agenda structure lines, like dates.
>> +This can be nil for unscaled, t for using the theme default, or a scaling number."
> 
> The last line should be reformatted to less than 80 characters, see:
> (info "(elisp) Documentation Tips")
> 
>> +  :group 'leuven)
> 
> You don't need ":group 'leuven" here since it defaults to the previous
> defgroup.
> 
> Best regards,
> Stefan Kangas
> 
> 
> 
> 

Message #28 received at 40759 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Fab Rice <posh18 <at> pirilampo.be>
Cc: 40759 <at> debbugs.gnu.org, stefan <at> marxist.se,
 Stefan Monnier <monnier <at> iro.umontreal.ca>, egrep <at> protonmail.ch,
 dgutov <at> yandex.ru
Subject: Re: bug#40759: 26.3; Update Leuven-theme in Emacs dev trunk
Date: Sat, 25 Apr 2020 11:49:50 +0300

> Date: Fri, 24 Apr 2020 14:09:56 +0200
> From: Fab Rice <posh18 <at> pirilampo.be>
> Cc: Serghei <egrep <at> protonmail.ch>, stefan <at> marxist.se, 40759 <at> debbugs.gnu.org,
>  dgutov <at> yandex.ru
> 
> +(defgroup leuven nil
> +  "Leuven theme options.
> +The theme has to be reloaded after changing anything in this group."
> +  :group 'faces)
> +
> +(defcustom leuven-scale-outline-headlines t
> +  "Scale `outline' (and `org') level-1 headlines.
> +This can be nil for unscaled, t for using the theme default, or a scaling
> +number."
> +  :type '(choice
> +          (const :tag "Unscaled" nil)
> +          (const :tag "Default provided by theme" t)
> +          (number :tag "Set scaling"))
> +  :group 'leuven)
> +
> +(defcustom leuven-scale-org-agenda-structure t
> +  "Scale Org agenda structure lines, like dates.
> +This can be nil for unscaled, t for using the theme default, or a scaling
> +number."
> +  :type '(choice
> +          (const :tag "Unscaled" nil)
> +          (const :tag "Default provided by theme" t)
> +          (number :tag "Set scaling")))

We don't have defcustom's in any other theme I see in etc/themes/.
Does 'disable-theme' know how to undo the defcustom's, i.e. makunbound
them so that they are no longer in the name-space? should it?  If so,
I think we should add such a feature, and only after that we will be
able to accept themes that define new user options.

> +(defun leuven-scale-font (control default-height)
> +  "Function for splicing optional font heights into face descriptions.
> +CONTROL can be a number, nil, or t.  When t, use DEFAULT-HEIGHT."
> +  (cond
> +   ((numberp control) (list :height control))
> +   ((eq t control) (list :height default-height))
> +   (t nil)))

Likewise with functions: should 'disable-theme' fmakunbound them?

Message #31 received at 40759 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Eli Zaretskii <eliz <at> gnu.org>, Fab Rice <posh18 <at> pirilampo.be>
Cc: 40759 <at> debbugs.gnu.org, stefan <at> marxist.se,
 Stefan Monnier <monnier <at> iro.umontreal.ca>, egrep <at> protonmail.ch
Subject: Re: bug#40759: 26.3; Update Leuven-theme in Emacs dev trunk
Date: Sat, 25 Apr 2020 17:36:31 +0300

On 25.04.2020 11:49, Eli Zaretskii wrote:
> We don't have defcustom's in any other theme I see in etc/themes/.
> Does 'disable-theme' know how to undo the defcustom's, i.e. makunbound
> them so that they are no longer in the name-space? should it?

I don't think it's a major problem, personally: these variables serve a 
purpose, and they don't hurt anybody by remaining defined.

Same with functions.

Message #34 received at 40759 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefan <at> marxist.se>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: Eli Zaretskii <eliz <at> gnu.org>, Fab Rice <posh18 <at> pirilampo.be>,
 40759 <at> debbugs.gnu.org, egrep <at> protonmail.ch,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#40759: 26.3; Update Leuven-theme in Emacs dev trunk
Date: Mon, 04 May 2020 14:35:34 +0200

Dmitry Gutov <dgutov <at> yandex.ru> writes:

> On 25.04.2020 11:49, Eli Zaretskii wrote:
>> We don't have defcustom's in any other theme I see in etc/themes/.
>> Does 'disable-theme' know how to undo the defcustom's, i.e. makunbound
>> them so that they are no longer in the name-space? should it?
>
> I don't think it's a major problem, personally: these variables serve a purpose,
> and they don't hurt anybody by remaining defined.
>
> Same with functions.

I agree with that conclusion, too.  I think it's fine that a theme
does this.  Maybe we could file a wishlist level request to add the
makunbound functionality if we think it's worth working on.

Is there anything left to discuss, or should I go ahead and push this?

Also, I think we would need Eli to decide if this should go to
emacs-27 or master.  Thanks.

Best regards,
Stefan Kangas

Message #37 received at 40759 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: 40759 <at> debbugs.gnu.org, posh18 <at> pirilampo.be, monnier <at> iro.umontreal.ca,
 egrep <at> protonmail.ch, dgutov <at> yandex.ru
Subject: Re: bug#40759: 26.3; Update Leuven-theme in Emacs dev trunk
Date: Mon, 04 May 2020 17:58:24 +0300

> From: Stefan Kangas <stefan <at> marxist.se>
> Cc: Eli Zaretskii <eliz <at> gnu.org>,  Fab Rice <posh18 <at> pirilampo.be>,
>   40759 <at> debbugs.gnu.org,  Stefan Monnier <monnier <at> iro.umontreal.ca>,
>   egrep <at> protonmail.ch
> Date: Mon, 04 May 2020 14:35:34 +0200
> 
> > I don't think it's a major problem, personally: these variables serve a purpose,
> > and they don't hurt anybody by remaining defined.
> >
> > Same with functions.
> 
> I agree with that conclusion, too.  I think it's fine that a theme
> does this.  Maybe we could file a wishlist level request to add the
> makunbound functionality if we think it's worth working on.
> 
> Is there anything left to discuss, or should I go ahead and push this?
> 
> Also, I think we would need Eli to decide if this should go to
> emacs-27 or master.  Thanks.

Frankly, I'm a bit afraid that such a significant deviation from the
practice of themes should be added to the release branch now.
Stefan's opinion notwithstanding, there can always be something we are
overlooking.

Message #40 received at 40759 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefan <at> marxist.se>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 40759 <at> debbugs.gnu.org, posh18 <at> pirilampo.be, monnier <at> iro.umontreal.ca,
 egrep <at> protonmail.ch, dgutov <at> yandex.ru
Subject: Re: bug#40759: 26.3; Update Leuven-theme in Emacs dev trunk
Date: Mon, 04 May 2020 17:11:34 +0200

Eli Zaretskii <eliz <at> gnu.org> writes:

> Frankly, I'm a bit afraid that such a significant deviation from the
> practice of themes should be added to the release branch now.
> Stefan's opinion notwithstanding, there can always be something we are
> overlooking.

Thank you.  I will allow a couple of days before I push this to
master, in case anyone has any further comments.  We should be able to
fix any problems that pop up in time before 28.1 is released.

Best regards,
Stefan Kangas

Message #43 received at 40759 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefan <at> marxist.se>
To: Fab Rice <posh18 <at> pirilampo.be>, Eli Zaretskii <eliz <at> gnu.org>
Cc: 40759 <at> debbugs.gnu.org, Serghei <egrep <at> protonmail.ch>, dgutov <at> yandex.ru
Subject: Re: bug#40759: 26.3; Update Leuven-theme in Emacs dev trunk
Date: Sat, 9 May 2020 12:48:53 -0400

Fab Rice <posh18 <at> pirilampo.be> writes:

> Here's an updated patch file, where I've fixed the issues pointed by Stefan.

Thank you.  I intended to push this today, but it seems like you haven't
merged the changes in Emacs into your version.

Could you please look into that and suggest how to best proceed?
Possibly you could just merge the changes and send a new patch.

I have marked some differences I could find below, but I think you would
need to review all this in full.

> -      (code-inline '(:foreground "#006400" :background "#FDFFF7"))
> +      (code-block '(:foreground "#000088" :background "#FFFFE0" :extend t))
> +      (code-inline '(:foreground "#006400" :background "#FDFFF7" :extend t))

The :extend attributes has been removed on purpose.
See emacs.git commit 58fb4c3e68a and etc/NEWS.27.

> +   ;; `(font-lock-doc-string-face ((,class (:foreground "#008000")))) ; XEmacs only, but is used for HTML exports from org2html (and not interactively)

This line was removed in emacs.git commit 16ec5f5f2ff.

>  ;; Local Variables:
> +;; no-byte-compile: t

This line was removed in emacs.git commit 530aa469a4d.

Sorry for not noticing any of this earlier.

Best regards,
Stefan Kangas

Message #46 received at 40759 <at> debbugs.gnu.org (full text, mbox):

From: Fab Rice <posh18 <at> pirilampo.be>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 40759 <at> debbugs.gnu.org,
 Serghei <egrep <at> protonmail.ch>, dgutov <at> yandex.ru
Subject: Re: bug#40759: 26.3; Update Leuven-theme in Emacs dev trunk
Date: Wed, 13 May 2020 19:36:39 +0200

[Message part 1 (text/plain, inline)]

Dear Stefan,

Le 2020-05-09 18:48, Stefan Kangas a écrit :
> Fab Rice <posh18 <at> pirilampo.be> writes:
> 
>> Here's an updated patch file, where I've fixed the issues pointed by 
>> Stefan.

I've pushed new changes, following your 3 (types of) remarks.

Please find it here, attached.

Thanks a lot!

Best regards,
Fabrice

[0001-Update-Leuven-theme.patch (text/x-diff, attachment)]

Message #49 received at 40759 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Fab Rice <posh18 <at> pirilampo.be>
Cc: Eli Zaretskii <eliz <at> gnu.org>, Stefan Kangas <stefan <at> marxist.se>,
 40759 <at> debbugs.gnu.org, Serghei <egrep <at> protonmail.ch>, dgutov <at> yandex.ru
Subject: Re: bug#40759: 26.3; Update Leuven-theme in Emacs dev trunk
Date: Sat, 08 Aug 2020 14:23:00 +0200

Fab Rice <posh18 <at> pirilampo.be> writes:

> I've pushed new changes, following your 3 (types of) remarks.
>
> Please find it here, attached.
>
> Thanks a lot!

As far as I can see, all the comments were taken into consideration in
this final version of the patch, but it was not applied?

So I've now done so in Emacs 28.1.  The patch didn't apply cleanly, so I
merged a few of the lines by hand.  Please look the results over to see
whether I messed up or not.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.