Since filedef config files are now stored under the "filedef" subdirectory of app->datadir, we need to add the subdirectory name when creating path from the corresponding app->configdir otherwise the file isn't found. You can view, comment on, or merge this pull request online at:
https://github.com/geany/geany/pull/1482
-- Commit Summary --
* Create correct path for filetype config files
-- File Changes --
M src/ui_utils.c (17)
-- Patch Links --
https://github.com/geany/geany/pull/1482.patch https://github.com/geany/geany/pull/1482.diff
Hmmm, on consideration, the whole idea of copying the global config is a bad idea, the user configs should only contain the fields they want to change, not hide the whole system config.
But of course just dropping the user in an empty config file is unhelpful too. Maybe after creating the new user config we should do a `s/^/# /g`.
This PR just fixes the paths.
Without the fix Geany does not copy the filetype files (albeit accidentally), so the user config doesn't hide the system config, so currently Geany does the right, but user unfriendly, thing.
"Fixing" the paths will make Geany do the wrong thing again so it shouldn't happen alone.
I just mean this PR is to fix simple bug in the code that caused a regression from previous versions. If you want to change the behaviour overall, you should make a new PR that properly removes/changes the affected code, leaving a bug in the source code is not really a good idea, IMO.
Without the fix Geany does not copy the filetype files (albeit accidentally), so the user config doesn't hide the system config, so currently Geany does the right, but user unfriendly, thing.
"Fixing" the paths will make Geany do the wrong thing again so it shouldn't happen alone.
The `filetypes.README` file inside the filedefs directory says:
``` Copy files from /usr/local/Cellar/geany/1.24.1/share/geany to this directory to overwrite them. To use the defaults, just delete the file in this directory. For more information read the documentation (in /usr/local/Cellar/geany/1.24.1/share/doc/geany/html/index.html or visit http://www.geany.org/). ```
and the code does just that for the user. Apart from overriding default settings another use case is just to look at the default configuration and without this patch nothing is displayed. Finally, this works the same way as editing `filetype_extensions.conf`, `filetypes.common` etc. always worked and the same overriding problem existed there too.
I'd say if the user wants to override just something, he'll have to delete the parts of the file he doesn't want to override.
I'd say if the user wants to override just something, he'll have to delete the parts of the file he doesn't want to override.
Thats why I suggested we add a`# ` to the front of each line (`s/^/# /g`). Everything is still visible and you can still see which things are commented out in the system file. Deletions are not needed.
Are user and system files always read both, such that user files can partially overwrite system files? If yes, then @elextr idea sounds very good to me, and if not it should be changed to behave that way.
systemd has something similar, called drop-ins, where only parts of the system unit file can be overwritten (it's also ini-style), so maybe this approach makes sense.
@kugel- I believe thats how its intended to work, and as far as I can tell, all the filetype file settings are read from `configh` then `config` if not found (where `configh` is the user home keyfile and `config` is the system one). Its spread across `filetypes.c`, `build.c` and `highlighting.c` so I may have missed something that isn't handled that way, but that would be a bug.
Certainly works for me, I have a filetype file that has only a few settings in it and they override only those system settings.
then config if not found
That's the key. I was asking if `config` is read unconditionally, and then `configh` is read (if found) to override `config` partially.
How else does partial overriding work?
Each setting in filetypes.c is read with [this macro](https://github.com/geany/geany/blob/9f4407ca29ac4dd6401fbc0688fb8c244158ae3d...) rather than reading all of one config file then all of the other, but the effect is the same, partial overriding.
And as you know build has a wonderful setting by setting overriding system :)
Highlighting is much more complex since the set of keys is not fixed, but seems to also read a setting from the user config if it exists and from the system one if not, @b4n could confirm.
Thats why I suggested we add a# to the front of each line (s/^/# /g). Everything is still visible and you can still see which things are commented out in the system file. Deletions are not needed.
But with the comments it looks to the user like something which is NOT used but in fact this is what is used in the global settings. So personally I'd keep it the way it is.
@techee, things that are not used by the global filetypes will have two #es at the start, things that are used will only have one. Its clear which are used in global and which are not.
@techee, things that are not used by the global filetypes will have two #es at the start, things that are used will only have one. Its clear which are used in global and which are not.
I'm still not convinced it's a good idea to change the behavior this way - editing `filetype_extensions.conf`, `filetypes.common`, `ignored.tags`, `snippets.conf` did always override the whole file. I just extended this so it worked for filetypes:
https://github.com/geany/geany/pull/491
and then broke it by
https://github.com/geany/geany/pull/485
Even with the broken behavior for filetypes, editing `filetype_extensions.conf`, `filetypes.common`, `ignored.tags`, `snippets.conf` still worked the same way because these weren't in a subdirectory.
Ok, the overloading issue is separated to #1552, LGBI
Closed #1482 via b7c4bb0b4d3022bb0102627cb6baa854cec93dd1.
Merged since @elextr separated out the main issue with this.
github-comments@lists.geany.org