[PATCH] Fix window width when line numbers present

[PATCH] Fix window width when line numbers present

[Matt Huszagh]

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

Hello,

This is a very small patch that I believe fixes a previous, incorrect
fix for computing the window text area when line numbers are
present. The previous fix seemed to assume that
display-line-numbers-width being nil meant that this means line numbers
are not present. However, it can also mean (as described in the
variable documentation) that the line number width is computed
dynamically.

Thanks
Matt


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-org.el-Fix-inline-image-width-calculation-when-line-.patch --]
[-- Type: text/x-patch, Size: 1194 bytes --]

From 5faaf45bf54c0cd3135b9c5a747c22797fe1d290 Mon Sep 17 00:00:00 2001
From: Matt Huszagh <huszaghmatt@gmail.com>
Date: Sun, 21 Nov 2021 10:30:30 -0800
Subject: [PATCH] org.el: Fix inline image width calculation when line numbers
 present

* lisp/org.el (org-display-inline-image--width): When
display-line-numbers-width is nil, the width is computed dynamically.
This does not mean that the line number width is necessarily 0.
---
 lisp/org.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/org.el b/lisp/org.el
index eeefb4af3..331bd9f65 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -16880,7 +16880,7 @@ buffer boundaries with possible narrowing."
                     (/ (or (and (bound-and-true-p visual-fill-column-mode)
                                 (or visual-fill-column-width auto-fill-function))
                            (when auto-fill-function fill-column)
-                           (- (window-text-width) (or display-line-numbers-width 0)))
+                           (- (window-text-width) (line-number-display-width)))
                        (float (window-total-width)))))
         width)))
    ((numberp org-image-actual-width)
-- 
2.31.1

Re: [PATCH] Fix window width when line numbers present

[Timothy]

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

Hi Matt,

> This is a very small patch that I believe fixes a previous, incorrect
> fix for computing the window text area when line numbers are
> present. The previous fix seemed to assume that
> display-line-numbers-width being nil meant that this means line numbers
> are not present. However, it can also mean (as described in the
> variable documentation) that the line number width is computed
> dynamically.

Thanks for this patch. I wasn’t aware of `line-number-display-width' when I wrote
that, but it looks like a better fit than my `(or ... 0)' statement. As such, I’ve
just applied your patch as cd3e138, tweaking the commit message to no longer go
over the maximum length :)

Looking at the docstring for `line-number-display-width', it seems that the
pixelwise mode may be more useful for our purposes, but that improvement can be
in a subsequent patch.

All the best,
Timothy

Re: [PATCH] Fix window width when line numbers present

[Nicolas Goaziou]

Hello,

Timothy <tecosaur@gmail.com> writes:

> Thanks for this patch. I wasn’t aware of `line-number-display-width' when I wrote
> that, but it looks like a better fit than my `(or ... 0)' statement. As such, I’ve
> just applied your patch as cd3e138, tweaking the commit message to no longer go
> over the maximum length :)

`line-number-display-width' is Emacs 26+. So I guess it is unfortunately
not acceptable on main branch.

Regards,
-- 
Nicolas Goaziou

Re: [PATCH] Fix window width when line numbers present

[Matt Huszagh]

Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

> `line-number-display-width' is Emacs 26+. So I guess it is unfortunately
> not acceptable on main branch.

Does org-mode not require 26+ even though the Emacs official release is
on 27?

Matt

Re: [PATCH] Fix window width when line numbers present

[Bastien]

Hi Matt,

Matt Huszagh <huszaghmatt@gmail.com> writes:

> Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:
>
>> `line-number-display-width' is Emacs 26+. So I guess it is unfortunately
>> not acceptable on main branch.
>
> Does org-mode not require 26+ even though the Emacs official release is
> on 27?

See https://orgmode.org/worg/org-maintenance.html#emacs-compatibility

-- 
 Bastien

Re: [PATCH] Fix window width when line numbers present

[Matt Huszagh]

Bastien <bzg@gnu.org> writes:

> See https://orgmode.org/worg/org-maintenance.html#emacs-compatibility

Ah, my mistake. Thanks for pointing me to that. So does this stay on
some other branch until the Emacs official release is at 28?

Matt

Re: [PATCH] Fix window width when line numbers present

[Bastien]

Hi Matt,

Matt Huszagh <huszaghmatt@gmail.com> writes:

> Bastien <bzg@gnu.org> writes:
>
>> See https://orgmode.org/worg/org-maintenance.html#emacs-compatibility
>
> Ah, my mistake. Thanks for pointing me to that. So does this stay on
> some other branch until the Emacs official release is at 28?

We can safely assume that Org 9.6 will be released after Emacs 28, so
let's use `line-number-display-width' on main.

-- 
 Bastien

Re: [PATCH] Fix window width when line numbers present

[Timothy]

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

Hi Bastien,

> We can safely assume that Org 9.6 will be released after Emacs 28, so
> let’s use `line-number-display-width’ on main.

So I take this to mean that we will now assume that Org 9.6+ will target Emacs
29? This seems reasonable to me, but doesn’t quite accord with how I read
<https://orgmode.org/worg/org-maintenance.html#emacs-compatibility>.

All the best,
Timothy

Re: [PATCH] Fix window width when line numbers present

[Bastien]

Hi Timothy,

Timothy <tecosaur@gmail.com> writes:

> So I take this to mean that we will now assume that Org 9.6+ will target Emacs
> 29? This seems reasonable to me, but doesn’t quite accord with how I read
> <https://orgmode.org/worg/org-maintenance.html#emacs-compatibility>.

Our commitment is that the latest Org version is compatible with the
last three stable versions of Emacs.

So when Emacs 28 and Org 9.6 are both out, we guarantee that Org is
compatible with Emacs 28, 27 and 26.

Does that explain it better?

Let me know if you think the text on Worg should be clarified.

-- 
 Bastien

Supported Emacs version (was: [PATCH] Fix window width when line numbers present)

[Timothy]

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

Hi Bastien,

> Our commitment is that the latest Org version is compatible with the
> last three stable versions of Emacs.
>
> So when Emacs 28 and Org 9.6 are both out, we guarantee that Org is
> compatible with Emacs 28, 27 and 26.
>
> Does that explain it better?

Thanks for clarifying. That’s perfectly clear, and re-reading the content of
<https://orgmode.org/worg/org-maintenance.html#emacs-compatibility> I think that
is too and I just must have read it strangely the first time??

On the topic of Emacs version compatibility, I’ve just taken a peek at
org-compat.el and notice that we still have code for Emacs < 24.4 compatibility.
I’m tempted to make some patches to remove all the Emacs < 26 compatibility code
if we no longer need it. What do you think?

All the best,
Timothy

Re: Supported Emacs version

[Bastien]

Hi Timothy,

Timothy <tecosaur@gmail.com> writes:

> I’m tempted to make some patches to remove all the Emacs < 26
> compatibility code if we no longer need it. What do you think?

We should NOT remove this code: many people probably use/need it.

Anything that helps keeping Org compatible with Emacs < 26 is 
still useful, the more compatible the better.

If for some reason, some backward-compatibility code becomes too
difficult to maintain then perhaps yes we can remove it, but not
before.

-- 
 Bastien

Re: Supported Emacs version

[Timothy]

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

Hi  Bastien,

>> I’m tempted to make some patches to remove all the Emacs < 26
>> compatibility code if we no longer need it. What do you think?
>
> We should NOT remove this code: many people probably use/need it.
>
> Anything that helps keeping Org compatible with Emacs < 26 is
> still useful, the more compatible the better.
>
> If for some reason, some backward-compatibility code becomes too
> difficult to maintain then perhaps yes we can remove it, but not
> before.

Hmm, I’m not entirely sure what to think here. On one hand, I very much agree
with you that it’s better if Org is compatible with older versions just by
having a bit of extra code left in org-compat.el. However, if many bits of Org
use Emacs 24/25/26 functions that /aren’t/ covered in org-compat.el (as I think is
currently the case) that leads to bits of Org that are covered by org-compat.el
and bits that aren’t, and so I imagine this currently produces inconsistently
buggy behaviour in old versions of Emacs.

If this were just a question of fully supporting older Emacs versions at no
extra cost, I’d be inclined to respond as you have, but I think the problem we
really have is whether it’s better to provide /partial/ support for older versions
of Emacs? Personally I’m more inclined towards an all-or-nothing approach.

All the best,
Timothy

Re: Supported Emacs version

[Bastien]

Hi Timothy,

Timothy <tecosaur@gmail.com> writes:

> Personally I’m more inclined towards an all-or-nothing approach.

I understand this inclination but I disagree.

Org is made of many areas and partial backward-compatibility can
still be useful.  When people report compatibility problems with
Emacs <26, we can guide them so that they enhance org-compat.el.

It is not because we don't promise compatibility for Emacs < 26
that we should prevent backward-compatibility for Emacs < 26, and
the all-or-nothing approach would actually prevent it.

I hope this is convincing enough :)

-- 
 Bastien

Re: Supported Emacs version

[Timothy]

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

Hi Bastien,

> Org is made of many areas and partial backward-compatibility can
> still be useful.  When people report compatibility problems with
> Emacs <26, we can guide them so that they enhance org-compat.el.
>
> It is not because we don’t promise compatibility for Emacs < 26
> that we should prevent backward-compatibility for Emacs < 26, and
> the all-or-nothing approach would actually prevent it.
>
> I hope this is convincing enough :)

It’s not the choice I’d make, but what matters more is that I now better
appreciate/understand the motivation for leaving those bit in.

Thanks for discussing this a bit :)

All the best,
Timothy

Re: Supported Emacs version

[Bastien]

Hi Timothy,

Timothy <tecosaur@gmail.com> writes:

> It’s not the choice I’d make, but what matters more is that I now better
> appreciate/understand the motivation for leaving those bit in.

well, this is always a trade-off: we try to stick to a "users' first*"
principle, while managing the burden for maintainers.  If suddenly the
burden of keeping this code is too heavy for maintainers and does not
really benefit the users, we might revise this decision.

> Thanks for discussing this a bit :)

Thanks for bringing this up!

* https://bzg.fr/en/the-software-maintainers-pledge :)

-- 
 Bastien

Re: [PATCH] Fix window width when line numbers present

[Ihor Radchenko]

Bastien <bzg@gnu.org> writes:

> Our commitment is that the latest Org version is compatible with the
> last three stable versions of Emacs.
>
> So when Emacs 28 and Org 9.6 are both out, we guarantee that Org is
> compatible with Emacs 28, 27 and 26.
>
> Does that explain it better?
>
> Let me know if you think the text on Worg should be clarified.

The text is mostly clear. However, there is a slight confusion about
actual current Emacs version vs. planned Emacs version.

Emacs 28 is have not been officially released yet. So, the "current
major version of Emacs" is 27 and we are theoretically committed to
Emacs 27, 26, and 25. Both on main and bugfix.

On the other hand, we do know that Org 9.5 (bugfix) is going to be
packaged together with Emacs 28 when it is out. Also, we do not yet know
the time when Emacs 28 is going to be released (it can be even, say, a
year from now). Will it happen before Org 9.6 (main)? Maybe. Maybe not.
And some people are already using main branch. Should we target
"current" Emacs 28 for main? Should we target Emacs version+1 as current
on main all the time or maybe just when new Emacs release branch is
created, but not yet released? It is not fully clear.

Best,
Ihor

Re: [PATCH] Fix window width when line numbers present

[Tim Cross]

Ihor Radchenko <yantar92@gmail.com> writes:

> Bastien <bzg@gnu.org> writes:
>
>> Our commitment is that the latest Org version is compatible with the
>> last three stable versions of Emacs.
>>
>> So when Emacs 28 and Org 9.6 are both out, we guarantee that Org is
>> compatible with Emacs 28, 27 and 26.
>>
>> Does that explain it better?
>>
>> Let me know if you think the text on Worg should be clarified.
>
> The text is mostly clear. However, there is a slight confusion about
> actual current Emacs version vs. planned Emacs version.
>
> Emacs 28 is have not been officially released yet. So, the "current
> major version of Emacs" is 27 and we are theoretically committed to
> Emacs 27, 26, and 25. Both on main and bugfix.
>
> On the other hand, we do know that Org 9.5 (bugfix) is going to be
> packaged together with Emacs 28 when it is out. Also, we do not yet know
> the time when Emacs 28 is going to be released (it can be even, say, a
> year from now). Will it happen before Org 9.6 (main)? Maybe. Maybe not.
> And some people are already using main branch. Should we target
> "current" Emacs 28 for main? Should we target Emacs version+1 as current
> on main all the time or maybe just when new Emacs release branch is
> created, but not yet released? It is not fully clear.
>

I agree it can be confusing. My interpretation -

As org 9.5 has been released and the latest Emacs version is still 27.2,
then org 9.5 needs to support 27.x, 26.x and 25.x.

Provided Org 9.6 is not released *before* Emacs 29, then it only needs
to support 29,.x, 28.x and 27.x. However, if org 9.6 is released before
Emacs 29, it has to support 28.x, 27.x and 26.x.

Of course, we could also interpret things differently. For example, we
could tie the support to the version of Emacs current at the time of
release. i.e. if we release org 9.6 before Emacs 29.1, then it would
have to support 28.x, 27.x and 26.x even if org 9.6 is what is released
in Emacs 29.

The difference in release schedules will mean this is always a little
challenging IMO.

Re: [PATCH] Fix window width when line numbers present

[Bastien]

Hi Ihor,

Ihor Radchenko <yantar92@gmail.com> writes:

> The text is mostly clear. However, there is a slight confusion about
> actual current Emacs version vs. planned Emacs version.

I've rephrased it a bit to focus on "releases", not "versions":

  We aim at keeping the latest stable version of Org compatible with
  the *last three major releases of Emacs*.

  For example, if the latest release of Emacs is 28.x, then you can
  assume that the latest release of Org is compatible with Emacs 28.x,
  27.x and 26.x, but not with Emacs 25.x.

> Should we target "current" Emacs 28 for main? Should we target Emacs
> version+1 as current on main all the time or maybe just when new
> Emacs release branch is created, but not yet released? It is not
> fully clear.

These paragraphes on Worg try to explicit reasonable expectations 
for users and are thus focusing on stable releases only.

Expectactions for development versions are another story: I am in
favor of not trying to commit to _any_ expectactions for Org's main.

For example, I would not mind a feature on main that relies on a new
feature in the Emacs development branch, as long as we manage to keep
the promise that the next Org stable is usable with the older Emacs
releases.

-- 
 Bastien

Re: [PATCH] Fix window width when line numbers present

[Tim Cross]

Bastien <bzg@gnu.org> writes:

> Hi Ihor,
>
> Ihor Radchenko <yantar92@gmail.com> writes:
>
>> The text is mostly clear. However, there is a slight confusion about
>> actual current Emacs version vs. planned Emacs version.
>
> I've rephrased it a bit to focus on "releases", not "versions":
>
>   We aim at keeping the latest stable version of Org compatible with
>   the *last three major releases of Emacs*.
>
>   For example, if the latest release of Emacs is 28.x, then you can
>   assume that the latest release of Org is compatible with Emacs 28.x,
>   27.x and 26.x, but not with Emacs 25.x.
>
>> Should we target "current" Emacs 28 for main? Should we target Emacs
>> version+1 as current on main all the time or maybe just when new
>> Emacs release branch is created, but not yet released? It is not
>> fully clear.
>
> These paragraphes on Worg try to explicit reasonable expectations 
> for users and are thus focusing on stable releases only.
>
> Expectactions for development versions are another story: I am in
> favor of not trying to commit to _any_ expectactions for Org's main.
>
> For example, I would not mind a feature on main that relies on a new
> feature in the Emacs development branch, as long as we manage to keep
> the promise that the next Org stable is usable with the older Emacs
> releases.

So what your saying is that if the current main branch relies on a
new feature in the current Emacs development branch, then org must also
have a compatibility function so that the feature works with at least
the last 2 released versions (possibly 3 since we don't know when the
new Emacs version will be released and we may need/want to release main
before the new Emacs version is released).


So basically, if you want to use a new feature in development Emacs, you
will also need to implement a compatibility version for the previous 2
or 3 Emacs versions. This probably sounds worse than it is in practice
because most of the time, once you have a compatibility implementation
for the previous version, it will likely work with the previous 2 or 3
versions. It further means that a new feature added into Emacs cannot be
used in org without a compatibility version until it has been in Emacs
for 3 versions. 

Re: [PATCH] Fix window width when line numbers present

[Timothy]

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

Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

> `line-number-display-width’ is Emacs 26+. So I guess it is unfortunately
> not acceptable on main branch.

Ooops, thanks for picking that up Nicolas. I see we actually have
┌────
│ (if (fboundp 'line-number-display-width)
│     (defalias 'org-line-number-display-width 'line-number-display-width)
│   (defun org-line-number-display-width (&rest _) 0))
└────
in org-compat.el, but I think that fallback definition could be improved — even
just `(or display-line-numbers-width 0)' would be better.

All the best,
Timothy

Re: [PATCH] Fix window width when line numbers present

[Timothy]

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

Oops, I forgot to mark this patch as applied for updates.orgmode.org.
Done in this message.