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

buffer: Fix ReloadSettings(true) for volatile filetype #3662

Merged
merged 6 commits into from
Feb 24, 2025

Conversation

JoeKar
Copy link
Collaborator

@JoeKar JoeKar commented Feb 12, 2025

As found out by @niten94 we didn't perform the callbacks in case we reload the filetype.

Fixes #3659

@JoeKar

This comment was marked as resolved.

@JoeKar JoeKar force-pushed the fix/reload-settings branch from 93c11d6 to 56659e3 Compare February 14, 2025 17:59
@JoeKar JoeKar force-pushed the fix/reload-settings branch from 31fe12c to bcd97ec Compare February 15, 2025 12:26
Copy link
Contributor

@niten94 niten94 left a comment

Choose a reason for hiding this comment

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

I've now read the code a lot that I understand it more and didn't test, but it seems to be fine other than some changes that are being done in an unrelated commit.

@JoeKar JoeKar force-pushed the fix/reload-settings branch 2 times, most recently from 885aae1 to 597d606 Compare February 16, 2025 19:09
We shall not overwrite a volatile set `filetype` provided as argument for micro:
`micro -filetype shell foo`
@JoeKar JoeKar force-pushed the fix/reload-settings branch from 597d606 to 3e81d91 Compare February 17, 2025 19:53
@JoeKar JoeKar force-pushed the fix/reload-settings branch from 3e81d91 to 882668b Compare February 19, 2025 19:35
* `UpdatePathGlobLocals()`
	* to apply the settings provided within e.g. "/etc/*": {}
* `UpdateFileTypeLocals()`
	* to apply the settings provided within e.g. "ft:shell": {}

We don't need to call `InitLocalSettings()` twice any longer.
This shall prevent unpredictable results caused by such a user configuration:

```
"ft:go" {
	"filetype": "c"
}
```
Like in NewBuffer(), we need to update glob-based local settings
before updating the filetype, since the filetype itself may be among those
glob-based local settings.
In `ReloadSettings()` the `filetype` can change upon globbed path given by
the `settings.json` or by identifying a different `filetype` based on the
file name given or pattern present inside the file.
To prevent further recursion caused by checking the `filetype` again, its
processing stops here and isn't considered in `DoSetOptionNative()`
once again where the callbacks are usually triggered.
Setting options directly in (h.)Buf.Settings without calling SetOption() or
SetOptionNative() is generally not the best idea, since it may not
trigger the needed side effects.
In particular, after zyedidia#3343,
directly setting `diffgutter` and `ruler` causes them not being tracked as
locally overridden per buffer, so if we run the `reload` command,
it unexpectedly replaces them with the default ones.
@JoeKar JoeKar force-pushed the fix/reload-settings branch from 882668b to ddc6051 Compare February 20, 2025 19:28
@JoeKar JoeKar merged commit c937479 into zyedidia:master Feb 24, 2025
6 checks passed
@JoeKar JoeKar deleted the fix/reload-settings branch February 24, 2025 17:01
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.

filetype command-line argument regression
3 participants