Skip to content

Comments

Relocate buffer view after setting options that affect it#3743

Merged
dmaluka merged 1 commit intomicro-editor:masterfrom
dmaluka:relocate-on-option-set
May 11, 2025
Merged

Relocate buffer view after setting options that affect it#3743
dmaluka merged 1 commit intomicro-editor:masterfrom
dmaluka:relocate-on-option-set

Conversation

@dmaluka
Copy link
Collaborator

@dmaluka dmaluka commented May 10, 2025

Whenever the user changes the value of an option that affects the calculation of display params in updateDisplayInfo(), we should immediately recalculate those params and relocate the buffer view accordingly.

For example: after enabling ruler via set ruler on, if the cursor is currently at the rightmost edge of the bufpane and softwrap is disabled, then the cursor becomes invisible (since it is now outside the view, since the buffer view width is decreased as a result of adding the ruler but StartCol is not updated accordingly), it only becomes visible again after the user types a character (or performs some other action that triggers updateDisplayInfo() + relocate). Fix it.

Somewhat related to #3741

Whenever the user changes the value of an option that affects the
calculation of display params in updateDisplayInfo(), we should
immediately recalculate those params and relocate the buffer view
accordingly.

For example: after enabling ruler via `set ruler on`, if the cursor is
currently at the rightmost edge of the bufpane and softwrap is disabled,
then the cursor becomes invisible (since it is now outside the view,
since the buffer view width is decreased as a result of adding the ruler
but StartCol is not updated accordingly), it only becomes visible again
after the user types a character (or performs some other action that
triggers updateDisplayInfo() + relocate). Fix it.
@dmaluka dmaluka force-pushed the relocate-on-option-set branch from 421d781 to 9f7dec7 Compare May 10, 2025 21:33
Comment on lines +65 to +66
if option == "diffgutter" || option == "ruler" || option == "scrollbar" ||
option == "statusline" {
Copy link
Member

Choose a reason for hiding this comment

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

Would fit for now, but I fear this will be forgotten in case we add further options affecting the window parameters.
So maybe move it into a method of the BufWindow e.g.:

		if isAffectingWindow(option) {
			w.updateDisplayInfo()
			w.Relocate()
		}
func (w *BufWindow) isAffectingWindow(option string) bool {
	if option == "diffgutter" || option == "ruler" || option == "scrollbar" ||
		option == "statusline" {
		return true
	}
	return false
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Maybe, OTOH we would still need to update that in 2 places. And it would make it less explicit which exact options we are dealing with here, in SetBuffer() (so e.g. if we for whatever reason add some of these options also to the above if, or to some new if that also does relocate, it will be not so obvious that we needlessly do relocate twice...)

Also note that technically infobar and keymenu affect window too, but we already handle them in a different way, by ensuring Tabs.Resize() after setting them (since they are global settings affecting all windows), which already calls updateDisplayInfo() and Relocate() for all affected windows, so we don't need to do that here.

Copy link
Member

Choose a reason for hiding this comment

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

Ok, convinced me. 👍

@dmaluka dmaluka merged commit 98ff79d into micro-editor:master May 11, 2025
6 checks passed
theredcmdcraft pushed a commit to theredcmdcraft/micro that referenced this pull request May 27, 2025
…or#3743)

Whenever the user changes the value of an option that affects the
calculation of display params in updateDisplayInfo(), we should
immediately recalculate those params and relocate the buffer view
accordingly.

For example: after enabling ruler via `set ruler on`, if the cursor is
currently at the rightmost edge of the bufpane and softwrap is disabled,
then the cursor becomes invisible (since it is now outside the view,
since the buffer view width is decreased as a result of adding the ruler
but StartCol is not updated accordingly), it only becomes visible again
after the user types a character (or performs some other action that
triggers updateDisplayInfo() + relocate). Fix it.
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.

2 participants