Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Rearranged configuration handling. #229

Merged
merged 1 commit into from
Aug 17, 2024
Merged

Conversation

gryf
Copy link
Contributor

@gryf gryf commented Aug 7, 2024

Till now, configuration has been done by somehow merging commandline options, reading config file and session files. Design have some flaws, which might occur with unexpected behaviour.

With this patch configuration will be sorted out and following order will be applied:

  1. If config file doesn't exists, bandcamp-dl will use defaults and try to write those defaults as a config file.
  2. If config does exists, it will be read and options it contain will be applied to the default config.
  3. In addition, if anything is passed through commandline, it will be used instead of the default/user configured options.

For example, default config could look as follows:

{
    "base-dir": "/home/username",
    "template": "%{artist}/%{album}/%{track} - %{title}",
    "overwrite": false,
    "no-art": false,
    "embed-art": false,
    "embed-lyrics": false,
    "group": false,
    "no-slugify": false,
    "ok-chars": "-_~",
    "space-char": "-",
    "ascii-only": false,
    "keep-spaces": false,
    "keep-upper": false,
    "no-confirm": false,
    "debug": false
}

user might have in his config file following keys:

{
    "base-dir": "/home/username/music",
    "keep-spaces": true,
    "keep-upper": true,
    "unrelated": "whatever"
}

and invoke a bandcamp-dl with options:

$ bandcamp-dl --base-dir /tmp https://bandcampurl

this in consequence will provide configuration for bandcamp-dl:

{
    "base-dir": "/tmp"
    "template": "%{artist}/%{album}/%{track} - %{title}",
    "overwrite": false,
    "no-art": false,
    "embed-art": false,
    "embed-lyrics": false,
    "group": false,
    "no-slugify": false,
    "ok-chars": "-_~",
    "space-char": "-",
    "ascii-only": false,
    "keep-spaces": true,
    "keep-upper": true,
    "no-confirm": false,
    "debug": false
}

in result, default base-dir will be "/tmp" because commandline options have override config option, keep-spaces and keep-upper have override hardcoded defaults.

Note, that options which are not expected will be ignored (like unrelated key in user config). Invalid json configuration will produce appropriate error message, and such config file will be ignored.

Session files are currently broken and will be subject for following fixes.

Till now, configuration has been done by somehow merging commandline
options, reading config file and session files. Design have some flaws,
which might occur with unexpected behaviour.

With this patch configuration will be sorted out and following order will
be applied:

1. If config file doesn't exists, bandcamp-dl will use defaults and try
   to write those defaults as a config file.
2. If config does exists, it will be read and options it contain will be
   applied to the default config.
3. In addition, if anything is passed through commandline, it will be
   used instead of the default/user configured options.

For example, default config could look as follows:

{
    "base-dir": "/home/username",
    "template": "%{artist}/%{album}/%{track} - %{title}",
    "overwrite": false,
    "no-art": false,
    "embed-art": false,
    "embed-lyrics": false,
    "group": false,
    "no-slugify": false,
    "ok-chars": "-_~",
    "space-char": "-",
    "ascii-only": false,
    "keep-spaces": false,
    "keep-upper": false,
    "no-confirm": false,
    "debug": false
}

user might have in his config file following keys:

{
    "base-dir": "/home/username/music",
    "keep-spaces": true,
    "keep-upper": true,
    "unrelated": "whatever"
}

and invoke a bandcamp-dl with options:

$ bandcamp-dl --base-dir /tmp https://bandcampurl

this in consequence will provide configuration for bandcamp-dl:

{
    "base-dir": "/tmp"
    "template": "%{artist}/%{album}/%{track} - %{title}",
    "overwrite": false,
    "no-art": false,
    "embed-art": false,
    "embed-lyrics": false,
    "group": false,
    "no-slugify": false,
    "ok-chars": "-_~",
    "space-char": "-",
    "ascii-only": false,
    "keep-spaces": true,
    "keep-upper": true,
    "no-confirm": false,
    "debug": false
}

in result, default base-dir will be "/tmp" because commandline options
have override config option, keep-spaces and keep-upper have override
hardcoded defaults.

Note, that options which are not expected will be ignored (like unrelated
key in user config). Invalid json configuration will produce appropriate
error message, and such config file will be ignored.

Session files are currently broken and will be subject for following
fixes.
@Evolution0
Copy link
Collaborator

Evolution0 commented Aug 10, 2024

Works for me, essentially what I had implemented on my end (though not with a class) then losing the ability to connect to bandcamp.com entirely derailed me.

If you want to version bump and switch to a dataclass go ahead, if there is anything else you want to update but are being held back by version just go for it and change the documentation to reflect that.

I'll wait a bit on this PR in case you would like to do that so it can go through as one PR.

@gryf
Copy link
Contributor Author

gryf commented Aug 12, 2024

It's fine. We can bump Python version and convert class to dataclass in separate PR. It's just internal representation of configuration - it will not affect user config anyway.

@Evolution0 Evolution0 merged commit f152b2f into iheanyi:master Aug 17, 2024
@kjake kjake mentioned this pull request Sep 11, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants