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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

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
internal/action/bufpane.go Outdated Show resolved Hide resolved
internal/display/statusline.go Show resolved Hide resolved
@@ -47,6 +47,12 @@ var statusInfo = map[string]func(*buffer.Buffer) string{
}
return ""
},
"overwrite": func(b *buffer.Buffer) string {
if b.GetActiveCursor().IsOverwriteMode {
return "del"
Copy link
Collaborator

Choose a reason for hiding this comment

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

"Delete" is not exactly what the overwrite mode is about. It is about overwrite aka replace, right?

I'm not sure which string should we display here.

I'm also wondering: can we be space-saving here and display a string only when in overwrite mode (similarly to $(modified))? Then maybe we could even add $(overwrite) to the default value of statusformatl.

Although again, I'm not sure which string should we display then. [overwrite] ? Anything shorter?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Even we find something, which we think it fits the needs...there is the one who likes to customize it anyway. 😉

Copy link
Contributor

Choose a reason for hiding this comment

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

I cannot come up with a better string, but I usually see "INS" displayed on the status bar when overwrite mode is enabled in GUI text editors. I don't think there could be a string with only symbols that can be recognized without confusion.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, I thought about [ins] too. I'm not entirely opposed to it, although arguably it would be a lie, since it is not the "insert mode" but the opposite of it (but hey, we can always say that it is "the mode that is triggered by pressing Insert, hence [ins]).

For reference, what some other editors display in their status lines, FWICS:

  • vim: -- REPLACE -- in overwrite mode, -- INSERT -- in normal insert mode
  • emacs: Ovwrt in overwrite mode, nothing in normal mode
  • geany: OVR in overwrite mode, INS in normal mode
  • nano doesn't seem to implement overwrite mode at all, the Insert key triggers something different

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'd prefer to keep the normal and default insert mode unmarked, as it is right now and only mark the replace/overwrite mode.
Unfortunately you don't like [>], because it is not self-explaining. 😉

How about [ow]...it is similar to [ro] or shall it be more explicit, but possibly longer?

Copy link
Collaborator

Choose a reason for hiding this comment

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

"ow" sounds like "ouch" or something.

[ovr]?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

over what?
I'm not really convinced.

[I]/[O] or [I]/[R], but then [I] needs to be displayed by default, otherwise [O] or [R] doesn't self explain.
Or as suggested by you initially just [overwrite], which is a bit longer, but explicit and really self explaining.

Copy link
Collaborator

Choose a reason for hiding this comment

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

[overwrite] is ok for me.

One more thought: maybe let's make [ro] and [overwrite] mutually exclusive? Since when the buffer is readonly, there is little point in informing the user that we are in overwrite mode, since the user cannot edit (either insert or overwrite) anything anyway.

i.e. a trivial change:

--- a/internal/display/statusline.go
+++ b/internal/display/statusline.go
@@ -48,7 +48,7 @@ var statusInfo = map[string]func(*buffer.Buffer) string{
 		return ""
 	},
 	"overwrite": func(b *buffer.Buffer) string {
-		if b.OverwriteMode {
+		if b.OverwriteMode && !b.Type.Readonly {
 			return "[overwrite] "
 		}
 		return ""

Copy link
Collaborator

Choose a reason for hiding this comment

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

since the user cannot edit (either insert or overwrite) anything anyway.

Well, actually currently the user can overwrite characters in a readonly buffer (try it: open help, press Insert and start typing characters), but that's a nasty bug to be fixed (#1805).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good catch, thanks.
#1805 needs to be fixed on its own.

@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
internal/buffer/buffer.go Outdated Show resolved Hide resolved
internal/action/bufpane.go Outdated Show resolved Hide resolved
internal/buffer/buffer.go Outdated Show resolved Hide resolved
internal/display/statusline.go Outdated Show resolved Hide resolved
runtime/help/options.md Show resolved Hide resolved
internal/action/bufpane.go Outdated Show resolved Hide resolved
@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
internal/buffer/buffer.go Outdated Show resolved Hide resolved
@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 from 8253c2f to 9960190 Compare January 23, 2025 17:32
@JoeKar JoeKar force-pushed the feature/cursor-overwrite-indicator branch 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.

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
4 participants