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.

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

fix: line numbers remain relative when helix loses focus #7955

Merged
merged 2 commits into from
Aug 29, 2023

Conversation

woojiq
Copy link
Contributor

@woojiq woojiq commented Aug 15, 2023

If line number = relative and a new window is opened in helix, lines inside unfocused windows will be absolute. This commit adds the same thing when helix becomes unfocused in a terminal emulator (or on screen in general).

I thought about introducing a new set_focus(self, bool) editor method, but saw that almost every Editor field was public so I decided to change it manually.

Small discussion: #7954

If `line number = relative` and a new window is opened in helix, lines inside unfocused windows will be `absolute`. This commit adds the same thing when helix becomes unfocused in a terminal emulator.
Copy link
Member

@pascalkuthe pascalkuthe left a comment

Choose a reason for hiding this comment

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

This potentially has some implementatiom overlap with #6858

@gabydd
Copy link
Member

gabydd commented Aug 15, 2023

We should settle on field so this doesn't conflict with #6858 otherwise looks good

@woojiq
Copy link
Contributor Author

woojiq commented Aug 15, 2023

Yes, it overlaps almost completly. In fact, if I rebase this PR on top of the mentioned, it will be a one-line solution? Should I rebase or maybe ask the author of the mentioned pr to add it to his pr?
I'm gonna partially rebase.

@woojiq woojiq marked this pull request as draft August 15, 2023 18:25
@woojiq
Copy link
Contributor Author

woojiq commented Aug 15, 2023

I changed the name to match the mentioned pr. This is a bit like crutch, but I don't want to change is_focus because it represents the focus among views, not focus in desktop/terminal. Therefore, I changed it only in gutters.

@woojiq woojiq marked this pull request as ready for review August 15, 2023 19:34
@gabydd
Copy link
Member

gabydd commented Aug 15, 2023

That looks good to me now👍

@archseer archseer merged commit b67d2c3 into helix-editor:master Aug 29, 2023
6 checks passed
@woojiq woojiq deleted the relative-line-unfocused branch August 29, 2023 08:42
@@ -171,7 +174,7 @@ impl EditorView {
view,
view.area,
theme,
is_focused,
is_focused & self.terminal_focused,
Copy link

Choose a reason for hiding this comment

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

The use of a single & here is a little confusing. Wouldn't it have been better to just use && here for clarity?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's already merged, I'm not going to create a PR for this small change.
Anyway, it is not difficult to learn bitwise operators and improve your programming knowledge a little.

Copy link

Choose a reason for hiding this comment

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

There's just no reason to add complexity by making this a bit-wise operation rather than a logical AND operation, especially in an open source project with this many moving pieces. I didn't expect a PR be made for this small change, was just pointing it out.

Copy link
Member

Choose a reason for hiding this comment

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

A bitwise operation on bool is no less clearer than a logical operation. I prefer the code as is.

dgkf pushed a commit to dgkf/helix that referenced this pull request Jan 30, 2024
…r#7955)

* fix: line numbers remain relative when helix loses focus

If `line number = relative` and a new window is opened in helix, lines inside unfocused windows will be `absolute`. This commit adds the same thing when helix becomes unfocused in a terminal emulator.

* partial rebase
mtoohey31 pushed a commit to mtoohey31/helix that referenced this pull request Jun 2, 2024
…r#7955)

* fix: line numbers remain relative when helix loses focus

If `line number = relative` and a new window is opened in helix, lines inside unfocused windows will be `absolute`. This commit adds the same thing when helix becomes unfocused in a terminal emulator.

* partial rebase
smortime pushed a commit to smortime/helix that referenced this pull request Jul 10, 2024
…r#7955)

* fix: line numbers remain relative when helix loses focus

If `line number = relative` and a new window is opened in helix, lines inside unfocused windows will be `absolute`. This commit adds the same thing when helix becomes unfocused in a terminal emulator.

* partial rebase
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants