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

Three-way merge configuration files #64

Open
3TUSK opened this issue Jul 1, 2023 · 3 comments
Open

Three-way merge configuration files #64

3TUSK opened this issue Jul 1, 2023 · 3 comments
Labels
enhancement New feature or request

Comments

@3TUSK
Copy link

3TUSK commented Jul 1, 2023

While revisiting issues discovered in our mod jam/convention event last year, I noticed this specific use-case:

  1. A mod has configuration file, let's say its initial values are
    {
       "foo": false, 
       "bar": false, 
       "baz": true
    }
  2. For the jam event only, the mod author wishes to ship a different default config, where all 3 options are enabled:
    {
       "foo": true, 
       "bar": true, 
       "baz": true
    }
  3. Some users wish to disable foo and go ahead to edit config on their own, i.e. their configs are now
    {
       "foo": false, 
       "bar": true, 
       "baz": true
    }
  4. For various reasons, at a later time, the mod author wishes to ship another default config, in which baz is now disabled by default:
    {
       "foo": true, 
       "bar": true, 
       "baz": false
    }

To my knowledge, current packwiz-installer can either keep the config in scenario 3, or use the config from scenario 4, but the mod author expects this instead (syntax highlighting is JavaScript - so we can have // comments):

{
  "foo": false, // user-modified value
  "bar": true, // This one is kept unchanged
  "baz": false // This one is based on latest changes
}

In other words, the mod author wishes to do a 3-way merge for configs from scenario 2, 3 and 4.

My understanding is that, to handle 3-way merging, we actually need to have a copy of config from scenario 2, for we need to generate diff between scenario 2 and 3, and between scenario 2 and 4. This original copy does not seem available, however; user might not keep a copy of the original, and packwiz does not store the original, either.

This feature may also require the modpack to be versioned. I haven't looked into this issue in deep, so I may be wrong on this.

@comp500 comp500 added the enhancement New feature or request label Jul 2, 2023
@comp500
Copy link
Member

comp500 commented Jul 2, 2023

Hmm.. this is interesting. A few things to note:

  • I don't plan to support configuration file formats directly, as this adds a significant amount of complexity and might not handle edge cases properly - such a 3-way merge would need to be done based on text (similar to Git diff / various IDEs)
  • The format packwiz-installer understands currently does not support any history (i.e. can't retrieve scenario 2) nor does packwiz-installer keep any local history
    • Keeping local history is unlikely to be implemented - this would require essentially replicating a version control system or enforcing that one is used, which I don't want to do, particularly with respect to disk usage
    • Retrieving history for files stored in the remote pack may be possible in future as I want to implement Git support
  • I feel like this behaviour is something that should be configurable - some files (e.g. quests, resources, configs that need to sync with the server) should never be merged unless the user explicitly opts to carry out a 2/3 way merge
    • For this I would probably add a per-file/folder flag that specifies which merge strategy should be used by default, as an extension of the current "preserve" functionality
    • This also needs to be careful to not annoy the user, particularly with changes that are automatically made by mods (e.g. missing fields, config format updates, .properties timestamps)

@3TUSK
Copy link
Author

3TUSK commented Jul 2, 2023

such a 3-way merge would need to be done based on text

I would agree.

I feel like this behaviour is something that should be configurable

Right now, for an entry in index.toml, we already have preserve to control whether we want to overwrite files. I am thinking that this could be expanded to allow a new value, say merge, which enables the merge behavior.

For files like quests, resources, we can already leave this false to always overwrite them.

@3TUSK
Copy link
Author

3TUSK commented Sep 18, 2024

I just realized that there is a more obvious use case - options.txt.

  1. Suppose you have a pack, and you ship a default options.txt for some modified keybindings.
  2. Some players added resource packs on their own, and these states are saved in options.txt.
  3. Later, you also added a default resource pack in options.txt.

This one is trickier because I don't think text-based merge can help you merge two arrays into one. That requires fully parsing options.txt, which adds complexity.

On the other hand, for options.txt specifically, we have specialized tools like Default Options to deal with.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants