Comments in config.ini are treated as options

[guihkx]

Platform

Operating system and version: Arch Linux
Legendary version: 0.20.34

Expected Behavior

Comments in ~/.config/legendary/config.ini should be completely ignored.

Current Behavior

Comments in ~/.config/legendary/config.ini are treated as options.

Steps to Reproduce

1. ~/.config/legendary/config.ini:

[legendary]
# foo
# foo
log_level = debug

2. legendary list-installed:

[LGDLFS] ERROR: Unable to read configuration file, please ensure that file is valid! (Error: While reading from '/home/gui/.config/legendary/config.ini' [line  4]: option '# foo' in section 'legendary' already exists)
[LGDLFS] WARNING: Continuing with blank config in safe-mode...

Installed games:
 * Rocket League® (App name: Sugar | Version: BC2_Update52+462850 | Platform: Windows | 30.51 GiB)

Total: 1

I'm also not allowed to add a comment at the top of the config file, i.e.:

# ~/.config/legendary/config.ini
[legendary]
# foo
# foo
log_level = debug

$ legendary list-installed
[LGDLFS] ERROR: Unable to read configuration file, please ensure that file is valid! (Error: File contains no section headers.
file: '/home/gui/.config/legendary/config.ini', line: 1
'# ~/.config/legendary/config.ini\n')
[LGDLFS] WARNING: Continuing with blank config in safe-mode...

Installed games:
 * Rocket League® (App name: Sugar | Version: BC2_Update52+462850 | Platform: Windows | 30.51 GiB)

Total: 1

[derrod]

This is a limitation of python's configparser.

[guihkx]

Are you sure about that?

From my limited understanding in Python, Legendary seems to be mistakenly limiting comments to only begin with the / character, by overriding the comment_prefixes option here:

legendary/legendary/lfs/lgndry.py

Line 50 in 3963382

self.config = LGDConf(comment_prefixes='/', allow_no_value=True)

I got the same parsing error when I replicated what Legendary does, but in interactive Python:

$ python3
Python 3.12.7 (main, Oct  1 2024, 11:15:50) [GCC 14.2.1 20240910] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from os import path
>>> from legendary.models.config import LGDConf
>>> cfg = LGDConf(comment_prefixes='/', allow_no_value=True)
>>> cfg.read(path.expanduser("~/.config/legendary/config.ini"))
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib/python3.12/site-packages/legendary/models/config.py", line 19, in read
    return super().read(filename)
           ^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/configparser.py", line 684, in read
    self._read(fp, filename)
  File "/usr/lib/python3.12/configparser.py", line 1064, in _read
    raise MissingSectionHeaderError(fpname, lineno, line)
configparser.MissingSectionHeaderError: File contains no section headers.
file: '/home/gui/.config/legendary/config.ini', line: 1
'# ~/.config/legendary/config.ini\n'

In contrast, I don't get any errors if I don't override comment_prefixes:

>>> cfg = LGDConf(allow_no_value=True)
>>> cfg.read(path.expanduser("~/.config/legendary/config.ini"))
['/home/gui/.config/legendary/config.ini']
>>> cfg.sections()
['legendary']
>>> 

The config.ini I used to test this:

# ~/.config/legendary/config.ini
[legendary]
# foo
# foo
log_level = debug

[derrod]

I honestly can't remember why that's set to /, but yeah that should include #...

[guihkx]

The change seems to be introduced by d842780 to fix #105, but some other common comment prefixes characters were not added...

[CommandMC]

The point of that change was to not treat comments as actual comments, as when writing to the config file, ConfigParser discards them

[guihkx]

Oh, I see. The issue I linked above seems to complain that after running legendary launch, the config.ini file would be overwritten, for some reason.

Is that behavior still prevent in the latest version? And if so, I wonder why?

[CommandMC]

Is that behavior still prevent in the latest version?

Yes:

legendary/legendary/cli.py

Line 3154 in 3963382

cli.core.exit()

legendary/legendary/core.py

Line 2154 in 3963382

self.lgd.save_config()

And if so, I wonder why?

This is intended behavior, to save any config modifications made by Legendary while it was running

[guihkx]

Thanks for clarifying. I wasn't aware that Legendary itself managed config.ini, I thought it was 100% user-managed.

So I think the only solution to this would be switching to a third-party ini parser that can actually retain comments upon saving the file, like ConfigObj.

But if that's not something you're interested at this time, feel free to close this. Otherwise, let me know so I can take a stab at implementing it.

[CommandMC]

Sorry, missed this

Assuming a 3rd-party parser has no downsides, I don't see why a PR moving to one wouldn't be accepted. Do note however that PRs might take a bit longer to get merged, as the maintainer of this project is busy with other things.

Personally, I don't really have an opinion on the matter. As mentioned, if a 3rd party parser is just as easy to use as Python's native one, it'd be fine by me to switch to it instead. On the other hand, to be completely honest, the problems you've outlined above do seem a little nitpicky (I guess I can understand wanting to add something to the top of the config file, but there's always the [Legendary] section; when do you ever need the exact same comment content in the same section multiple times?)

Don't get me wrong, your workflow's your workflow and if something's stopping you from having it work how you want it to, feel free to change things. I'm just wondering whether it might be easier to change your workflow slightly and save yourself the effort of porting this project to another .ini parser

[Kobi-Blade]

This is just a case of misuse of configparser, that module is to create and read *.ini files, not to update *.ini files.

To update the *.ini file you supposed to be using configupdater or even configobj as suggested above.