Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Use native line numbers in Emacs 26 #10675

Closed
wants to merge 6 commits into from
Closed

Use native line numbers in Emacs 26 #10675

wants to merge 6 commits into from

Conversation

bmag
Copy link
Collaborator

@bmag bmag commented May 14, 2018

Note to other maintainers: don't merge this yet, I might need to make a few more changes

Emacs 26.1 will introduce a new minor mode for native line numbers: display-line-numbers-mode. It is meant to replace linum and friends (including linum-relative). This PR causes Spacemacs to use display-line-numbers-mode in Emacs 26, and linum-mode in Emacs 25. If nlinum layer is enabled by the user, then nlinum-mode will be used instead of display-line-numbers-mode. Users of nlinum layer are recommended to use display-line-numbers-mode in Emacs 26.

There's a small issue that display-line-numbers-type is a "global" variable (not buffer-local), so toggling between absolute and relative line numbers in one buffer may affect other buffers. If you test this PR and want to comment about how it behaves, please do.

Currently, the SPC t n and SPC t r toggles for the three different line numbers solutions behave differently. I'm not sure which behavior is the best, so I'd like to hear what other people prefer. The different behaviors are as follows:

(this summary shows state transitions for line numbers status. "off" means no line numbers are shown, "absolute" means absolute line numbers are shown, "relative" means relative line numbers are shown)
linum:

  • SPC t n: (state before key press --> state after key press)
    • off (variant 1) --> absolute
    • off (variant 2) --> relative
    • absolute --> off (variant 1)
    • relative --> off (variant 2)
  • SPC t r:
    • off (variant 1) --> relative
    • off (variant 2) --> off (variant 1)
    • absolute --> relative
    • relative --> off (variant 1)

nlinum:

  • SPC t n:
    • off (variant 1) --> absolute
    • off (variant 2) --> relative
    • absolute --> off (variant 1)
    • relative --> off (variant 2)
  • SPC t r:
    • off (variant 1) --> off (variant 2)
    • off (variant 2) --> off (variant 1)
    • absolute --> relative
    • relative --> absolute

display-line-numbers:

  • SPC t n:
    • off --> absolute
    • absolute --> off
    • relative --> absolute
  • SPC t r:
    • off --> relative
    • absolute --> relative
    • relative --> off

@bmag bmag mentioned this pull request May 14, 2018
Closed
@jsoo1
Copy link

jsoo1 commented May 14, 2018

Is there any way to keep the same state transitions in native line numbers as in nlinum?

@jsoo1
Copy link

jsoo1 commented May 14, 2018

Also, awesome work and thank you.

jsoo1
jsoo1 reviewed May 14, 2018
View reviewed changes
Copy link

@jsoo1 jsoo1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll test this out today. Thanks again!

layers/+spacemacs/spacemacs-defaults/packages.el
@@ -17,14 +17,16 @@
(conf-mode :location built-in)
(dired :location built-in)
(dired-x :location built-in)
(display-line-numbers :location built-in
:toggle (version<= "26" emacs-version))
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is display-line-numbers stable in versions < 26.1?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't really matter. The first 26.x release will be 26.1, and the development process already reached RC1 whereemacs-version says "26.1". Any Emacs build that calls itself version 26.0.x is old and irrelevant by now.

Copy link

@jsoo1 jsoo1 May 15, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. I just want to make sure that if someone decided to use some version 26.0.x in the future for any reason it wouldn’t break for them.

@jsoo1
Copy link

jsoo1 commented May 14, 2018

So far it looks good. I was able to use relative numbers in a large file with no problem. Tested on GUI and daemon (frame and terminal). Both look good. Some feedback:

  1. The changes to the state transitions are a little jarring. I would prefer if we could keep the old transitions.
  2. Some modes that don't normally have line numbers have line numbers (Magit, in particular).

@syl20bnr
Copy link
Owner

syl20bnr commented May 15, 2018 via email

@syl20bnr
Copy link
Owner

syl20bnr commented May 15, 2018 via email •
edited
Loading

@syl20bnr
Copy link
Owner

And let's make the relative toggle key binding be SPC t N instead of SPC t r :-)

@jsoo1
Copy link

jsoo1 commented May 15, 2018

Aww man! I liked SPC t r...

@bmag
Copy link
Collaborator Author

bmag commented May 15, 2018

Some modes that don't normally have line numbers have line numbers (Magit, in particular).

Because we have a bug: setting dotspacemacs-line-numbers to t or relative enables them in all modes, instead of only in prog-mode and text-mode derivatives (as the documentation claims). I have a fix for it that I will push later.

By the way, turns out magit-mode itself disables linum-mode and nlinum-mode in Magit buffers. If it didn't do that, the same bug would happen with linum-mode as well.

Users of nlinum layer are recommended to use display-line-numbers-mode in Emacs 26.

Let's deprecate it for Emacs 26+ users

You sure about it? I didn't deprecate the layer because it is opt-in, but I'm ok with deprecating it if you're sure.

@bmag
Copy link
Collaborator Author

bmag commented May 15, 2018

Made a small correction to the toggles (so toggling in one buffer won't affect state of other buffers) and fixed the bug that @jsoo1 and I found (see 34ba605 for details).

cc @deb0ch

@deb0ch
Copy link
Contributor

deb0ch commented May 15, 2018

@bmag you called me ?

So excited about this PR, learning at the same time the existence of native line numers in Emacs 26 and that Spacemacs will integrate them 😺

@syl20bnr
Copy link
Owner

syl20bnr commented May 15, 2018 via email

@kommen
Copy link
Contributor

kommen commented May 21, 2018

I'm testing this in my spacemacs config with Emacs 26.1rc1 and so far it works great, thank you @bmag!
As a user, the state transition mappings work for me.
I just noticed one very minor issue though: when starting emacs, on the *spacemacs* buffer the line numbers a briefly visible, leading to a line wrap in the header. See

screen shot 2018-05-21 at 18 35 59

@bmag
Copy link
Collaborator Author

bmag commented May 21, 2018

when starting emacs, on the *spacemacs* buffer the line numbers a briefly visible, leading to a line wrap in the header.

I also see them enabled briefly during startup, and after a short while they are disabled (before startup finishes). Don't know why yet. Not sure what you meant about line wrap, though, isn't it normal (except the line numbers)?

@kommen
Copy link
Contributor

kommen commented May 21, 2018

@bmag yes, after a short while they are disabled again for me as well.

I mean that while the line numbers are visible, the "emacs)" part of "(spacemacs)" in the top right wraps, as visible in the screenshot. This seems to be a result of the right-aligning not respecting the width of the line numbers?

@bmag
Copy link
Collaborator Author

bmag commented May 21, 2018

@kommen ok, now I see it. The native line numbers use their own new margin-like space, maybe the home buffer's code doesn't respect that.

Anyway, I pushed a fix for it now, thanks.

@kommen
Copy link
Contributor

kommen commented May 21, 2018

@bmag thanks, also fixes it for me.

bmag added 6 commits May 28, 2018 19:00
@bmag
define-toggle: add option for custom off-message
72bd99b
@bmag
Add support for native line numbers in Emacs 26
369d557 
Emacs 26.1 will introduce a new native line numbers feature:
"display-line-numbers". It includes relative line numbers, is faster than
current linum-mode, and doesn't use the margin area (it has its own area). So
yeah, we want to use the new feature when possible.

No changes are required on the user side, except for Emacs 26 users are
recommended to remove nlinum layer from their configuration (if they have
enabled it).

With this change:
- Emacs 26:
  - uses display-line-numbers by default.
  - linum and linum-relative packages are excluded.
- Emacs 25:
  - does NOT use display-line-numbers.
  - continues to use linum and linum-relative.
- nlinum layer:
  - can still be used as before in Emacs 25.
  - is NOT recommended in Emacs 26, but can be used.
  - when enabled, excludes display-line-numbers.
@bmag
Line numbers: use correct buffer-local variables for local toggles
435bd95
@bmag
Line numbers bug fix: only in prog-mode and text-mode by default
3c73c8f 
Fixes a bug where setting `dostpacemacs-line-numbers` to `t` or `relative`
enabled line numbers in every buffer, instead of only in buffers that derive
from prog-mode and text-mode.

Likewise fixes a bug where specifying `:enabled-for-modes nil` (or not
specifying `:enabled-for-modes` at all) in `dotspacemacs-line-numbers` settings
meant "enable in all modes" instead of "enable in modes derived from prog-mode
or text-mode".

Also adds a way for users to enable line numbers in *all* buffers.
@bmag
Fix line numbers briefly shown in startup buffer
f367561
@bmag
line numbers: remove check for special buffer
8fb0213 
All our current checks should be enough to enable line numbers only where it
makes sense. Disabling in all special buffers is not necessary.
@bmag
Copy link
Collaborator Author

bmag commented May 28, 2018

Rebased and cherry picked into develop.

I kept the toggles as described in my initial post, since the majority (including me) seems to prefer it that way. Also removed the check for special buffers, since I'm pretty sure we don't need it anymore.

After pushing the update I see I forgot to deprecate nlinum for Emacs 26 users. @syl20bnr how do we deprecate an entire layer? Change nlinum-packages so it is empty when Emacs 26 is detected? Also issue a warning?

@bmag bmag closed this May 28, 2018
@bmag bmag removed the DO NOT MERGE label May 28, 2018
@reionwong
Copy link

wow, this is fast !!!

@syl20bnr
Copy link
Owner

syl20bnr commented Jun 2, 2018 via email •
edited
Loading

@duianto duianto mentioned this pull request Nov 26, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jsoo1 jsoo1 jsoo1 left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@bmag @jsoo1 @syl20bnr @deb0ch @kommen @reionwong