Skip to content

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

Merged
JoeKar merged 6 commits intomicro-editor:masterfrom
JoeKar:fix/reload-settings
Feb 24, 2025
Merged

buffer: Fix ReloadSettings(true) for volatile filetype#3662
JoeKar merged 6 commits intomicro-editor:masterfrom
JoeKar:fix/reload-settings

Conversation

@JoeKar
Copy link
Member

@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 micro-editor#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 micro-editor: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