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

core: add option weechat.look.save_config_changed_only (issue #19) #1371

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sim642
Copy link
Member

@sim642 sim642 commented Jul 4, 2019

This PR contains a simple change to save changed options only as an attempt at fixing issue #19. It works for normal options but has many problems and far from production ready.

Use at your own risk! While experimenting, I managed to completely break WeeChat (and the existing configuration) in multiple ways (see problems below).

Problems

  • config_file_new_section/weechat_config_new_section allows specifying a callback_write which, if specified, is used instead of the default config section writer. Even though the default config section writer can ignore unchanged options, each section with a custom writer would also have to duplicate that logic for consistent behavior.

    Luckily most WeeChat core, plugins and scripts that create their own config files and sections don't use a custom write callback. Of those that do, most write no options by default, e.g. the filters section has its own write callback but by default no filters exist so no non-default options get written anyway. The remaining cases need to be carefully analyzed (and potentially somehow fixed).

  • bars section consists of bar options. They are unusual because when they're read from the config file, the read values don't become option values but instead option default values. This means that they appear as unchanged from default values, even though the user might have changed them previously. Writing such config again would omit those options and thus user customizations!

    It also affects default bars because they get created once and later are indistinguishable from any custom bars, breaking even the default bars. This PR uses a condition to always write all options in the bars section to avoid this issue.

    Similar problems might affect other sections, e.g. triggers.

  • key* sections consists of key bindings for different contexts. They are written using special write callback which (in this PR) doesn't ignore unchanged key bindings. If unchanged key bindings were not written, no default keys would exist because the default keys are all written to the config initially. It would make WeeChat completely unusable because even enter is unbound and nothing can be sent/executed.

@codecov-io
Copy link

codecov-io commented Jul 4, 2019

Codecov Report

Merging #1371 into master will increase coverage by <.01%.
The diff coverage is 40%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1371      +/-   ##
==========================================
+ Coverage    25.9%   25.91%   +<.01%     
==========================================
  Files         198      198              
  Lines       81174    81179       +5     
==========================================
+ Hits        21032    21034       +2     
- Misses      60142    60145       +3
Impacted Files Coverage Δ
src/core/wee-config.c 45.53% <100%> (+0.05%) ⬆️
src/core/wee-config-file.c 49.4% <25%> (-0.07%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 789fa97...878c4aa. Read the comment docs.

@flashcode flashcode added the feature New feature request label Jul 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants