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

statusline: Provide overwrite mode indicator #3620

Merged
merged 2 commits into from
Jan 31, 2025

Conversation

JoeKar
Copy link
Collaborator

@JoeKar JoeKar commented Jan 20, 2025

We can store the overwrite state in the Buffer instead of the BufPane, which makes it easier to be accessed from the statusline via the statusformatl option.

Closes #3155

@JoeKar JoeKar marked this pull request as draft January 20, 2025 21:17
@dmaluka
Copy link
Collaborator

dmaluka commented Jan 20, 2025

We can store the overwrite state in the cursor instead of the bufpane

I think abusing Cursor for that is hacky and wrong, and on top of that completely unnecessary, since we can just store it in Buffer (but not in SharedBuffer, that's the key).

That's the trick we use with LastSearch or HighlightSearch, for example. They are provided by buffer, not by bufpane, so they can be used by display, and yet they are effectively per bufpane, i.e. each bufpane has its own search state even if it shares its buffer with other bufpanes.

@JoeKar
Copy link
Collaborator Author

JoeKar commented Jan 20, 2025

we can just store it in Buffer (but not in SharedBuffer, that's the key).

Good point, works like a charm!

@JoeKar JoeKar force-pushed the feature/cursor-overwrite-indicator branch from 67c67e8 to 40a7486 Compare January 20, 2025 22:57
@JoeKar JoeKar force-pushed the feature/cursor-overwrite-indicator branch from 40a7486 to 2813b5f Compare January 21, 2025 19:13
@JoeKar JoeKar marked this pull request as ready for review January 21, 2025 19:15
@JoeKar JoeKar force-pushed the feature/cursor-overwrite-indicator branch 2 times, most recently from 0abf3d7 to 8253c2f Compare January 22, 2025 18:56
@JoeKar JoeKar force-pushed the feature/cursor-overwrite-indicator branch 2 times, most recently from 9960190 to c1f4e74 Compare January 23, 2025 20:18
@matthias314
Copy link
Contributor

Sorry for chiming in late, and please ignore this comment if you think it just makes things more complicated.

I personally find that [overwrite] takes up quite a lot of space in the statusline, at least for a terminal window that is, say, 80 characters wide. What about shortening it to [over]? Something completely different that came to my mind would be to display the cursor position is reverse (foreground and background color exchanged) if in overwrite more.

@JoeKar
Copy link
Collaborator Author

JoeKar commented Jan 24, 2025

I personally find that [overwrite] takes up quite a lot of space in the statusline

We're aware of the fact that it actually requires a lot space within the statusline and discussed it here:
#3620 (comment)

Unfortunately we didn't came up with a better, shorter and self explaining suggestion.

Something completely different that came to my mind would be to display the cursor position is reverse (foreground and background color exchanged) if in overwrite more.

Hm, something to consider.

@niten94
Copy link
Contributor

niten94 commented Jan 25, 2025

Something completely different that came to my mind would be to display the cursor position is reverse (foreground and background color exchanged) if in overwrite more.

There are cases where the cursor is displayed as a box so relying only on cursor appearance to indicate overwrite mode cannot be done, but it is an additional feature that may be good with other cursor styles.

@JoeKar
Copy link
Collaborator Author

JoeKar commented Jan 25, 2025

Ok, how to proceed?
Shall we make a vote for a shorter indicator text first?

@niten94
Copy link
Contributor

niten94 commented Jan 25, 2025

I do not know if it is better to make a poll, but I think we shouldn't change the text or add a feature like @matthias314 said if there is no clear agreement. I am not good with decisions, so I will just leave what I think in this comment and let others or just maintainers decide (once again).

Overwrite mode being indicated in the status line may be already enough, so I think it is better to display box cursor on overwrite mode only if there is demand. (misunderstanding)

I am fine with the indicator text being changed to [ins] or [over] ([ovr] seems weird). Someone may be confused a bit at first if short indicator text is used and overwrite is accidentally enabled. However, I think micro users can usually realize with such text that overwrite mode was enabled and remember the meaning.

It is just nice if the string could be short, but the important part to me is that anyone can easily realize it and remember the meaning.

@matthias314
Copy link
Contributor

relying only on cursor appearance to indicate overwrite mode cannot be done

Just to clarify: I didn't mean to change the cursor itself, but rather the way the cursor position is displayed in the statusline. (There seems to be a consensus now to go in a different direction, and that's fine.)

@dmaluka
Copy link
Collaborator

dmaluka commented Jan 25, 2025

Another option is [ovwrt] like in emacs. A bit shorter than [overwrite], and rather unambiguous.

Regarding changing the way the cursor position is displayed in the statusline, the problem is (at least) that the user can arbitrarily override the format of the statusline by changing the statusformatl option. So it may not include $(line),$(col) at all, or for example it may not include $(line) at all but may include $(col) 15 times (why not?), and so on.

@niten94
Copy link
Contributor

niten94 commented Jan 25, 2025

Just to clarify: I didn't mean to change the cursor itself, but rather the way the cursor position is displayed in the statusline. (There seems to be a consensus now to go in a different direction, and that's fine.)

Ah... I am sorry that I did not read properly.

Regarding changing the way the cursor position is displayed in the statusline, the problem is (at least) that the user can arbitrarily override the format of the statusline by changing the statusformatl option.

I may have caused a misunderstanding so I'll restate the suggested feature, but it was displaying the cursor position with the background and foreground color exchanged. Thinking about specific problems:

  • It may look weird if done only with $(line) and $(col), and functions like status.vcol have to be supported.
  • Probably excessive if the start and end of the part displayed like such is set in statusformatl.

The only action to do now probably remains with deciding or voting on a shorter string.

My perspective may be unusal so please ignore this part if not useful, but "over" is used in text structure and not abbreviated so [over] looks weird to me now. The only suggested text that look fine to me are [ins] and [ovr].

@JoeKar
Copy link
Collaborator Author

JoeKar commented Jan 26, 2025

Ok, so [ovwrt] only (nothing for the usual insert) or [ins] + [ovr], where we've [ins] displayed the most of the time?

@JoeKar JoeKar force-pushed the feature/cursor-overwrite-indicator branch from c1f4e74 to e7aacdb Compare January 28, 2025 20:36
@JoeKar
Copy link
Collaborator Author

JoeKar commented Jan 28, 2025

[...] or [ins] + [ovr], where we've [ins] displayed the most of the time?

Updated with this, because it doesn't resize the statusline...less flickering.

@dmaluka
Copy link
Collaborator

dmaluka commented Jan 28, 2025

Updated with this, because it doesn't resize the statusline...less flickering.

So now it adds [ins] (arguably not very useful most of the time) to the statusline by default?

@niten94
Copy link
Contributor

niten94 commented Jan 29, 2025

Sorry, I should have specified what I meant. I also realized there may be problems when using [ins] in any way. There at least seems to be no objections with [ovr].

Updated with this, because it doesn't resize the statusline...less flickering.

So now it adds [ins] (arguably not very useful most of the time) to the statusline by default?

Overwrite mode is not usually enabled, so I agree too that it wouldn't be useful much (and wastes limited space). If there are no use cases where overwrite mode would be toggled fast a lot, then I don't think anyone will mind the status line flickering.

@dmaluka
Copy link
Collaborator

dmaluka commented Jan 29, 2025

Maybe [ov]? Same number of characters as [ro]. :)

Or [ovwr]...

@niten94
Copy link
Contributor

niten94 commented Jan 29, 2025

Both and [ovwrt] seems fine, but at this point I don't have good reasons to choose a particular string. I have mentioned a lot of what I think, so I think I'll now just leave the string to be decided.

@JoeKar
Copy link
Collaborator Author

JoeKar commented Jan 29, 2025

So now it adds [ins] (arguably not very useful most of the time) to the statusline by default?

Yes.

Overwrite mode is not usually enabled, so I agree too that it wouldn't be useful much (and wastes limited space). If there are no use cases where overwrite mode would be toggled fast a lot, then I don't think anyone will mind the status line flickering.

But it doesn't seem to bother anyone but me.
So if we really don't need [ins] then we can remove it and stay with something short for overwrite only.
The main thing is that we close this PR soon.

@JoeKar JoeKar force-pushed the feature/cursor-overwrite-indicator branch from e7aacdb to bc2df88 Compare January 29, 2025 16:24
@Andriamanitra
Copy link
Contributor

I don't really care what it says in overwrite mode as that is not visible under normal circumstances, but I don't like taking away space on the statusline in normal input mode with [ins]. [ins] can also be misinterpreted as "Insert has been pressed, so overwrite mode is active".

@dmaluka
Copy link
Collaborator

dmaluka commented Jan 30, 2025

The latest version (with [ov]) looks good to me. But I don't really care whether it would be [ov], [ovr], [ovwr], [ovwrt] or [overwrite], so perhaps someone prefers to change it to something else from this list, before we merge it?

@dmaluka
Copy link
Collaborator

dmaluka commented Jan 30, 2025

Actually after a bit more thinking, [ovwr] seems better (more explanatory) than [ov]?

@JoeKar
Copy link
Collaborator Author

JoeKar commented Jan 30, 2025

Then we go with the first two letters of over and write [ovwr].
Hopefully the last change here 🙏.

@JoeKar JoeKar force-pushed the feature/cursor-overwrite-indicator branch from bc2df88 to 57a6e81 Compare January 30, 2025 19:19
@JoeKar JoeKar merged commit e4b0ad7 into zyedidia:master Jan 31, 2025
6 checks passed
@JoeKar JoeKar deleted the feature/cursor-overwrite-indicator branch January 31, 2025 16:59
@Bellavene
Copy link

Bellavene commented Feb 3, 2025

Better to add a status line configuration option. Like I use "statusformatl": "$(icons.FileIcon)$(filename) $(modified)($(line),$(col))" Would be great to wrap ($(line),$(col))" with something like $(insert.inverted$(line),$(col))" so the line and colon numbers part would invert or colored red when insert mode is enabled. Also, this way you could define any characters you want.

Btw, there is a line $(overwrite) available. It does nothing for me, what is it for then?

@JoeKar
Copy link
Collaborator Author

JoeKar commented Feb 3, 2025

Btw, there is a line $(overwrite) available. It does nothing for me, what is it for then?

With this merged PR $(overwrite) can and is used to mark the [ovwr] being active after the ins-key was pressed.

@Bellavene
Copy link

Bellavene commented Feb 3, 2025

Great, but maybe allow to select any part of the status line to indicate it? The same would be also great for $(modified), a + sign is ok, but ability to change the color of the $(filename) when modified would be better.

@JoeKar
Copy link
Collaborator Author

JoeKar commented Feb 3, 2025

Maybe, but please then as a different feature request.

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.

[feature request] insert/overwrite indicator
6 participants