Leading `#` turnes to `%23` in config file

[mmahmoudian]

Flameshot version Flameshot v0.10.1 (aca0db96) Compiled with Qt 5.15.2 linux: 5.10.68-1-MANJARO

Describe the bug

When commenting a line in the config file and after running flameshot gui once, the config file will be automatically sorted and (this is expected as we discussed before) and the # would turn into it's hex code (%23). I believe it is confusing for the user and also this will break syntax highlighting of any texteditor.

To Reproduce

1. flameshot
2. Add a # to the begining of a line in the config file
3. flameshot gui
4. get out of Flamehsot (cancel or capture)
5. check the config file again

Expected behavior

I expect the # to stay as the same character.

System Information

System:
  Host: precisiontower Kernel: 5.10.68-1-MANJARO x86_64 bits: 64 
  Desktop: KDE Plasma 5.22.5 Distro: Manjaro Linux 
Graphics:
  Device-1: Intel CometLake-S GT2 [UHD Graphics 630] driver: i915 v: kernel 
  Device-2: AMD Lexa XT [Radeon PRO WX 3200] driver: amdgpu v: kernel 
  Display: x11 server: X.Org 1.20.13 driver: loaded: amdgpu,ati,modesetting 
  resolution: 1: 1920x1080~60Hz 2: 1080x1920~60Hz 
  OpenGL: renderer: AMD Radeon Pro WX 3200 Series (POLARIS12 DRM 3.40.0 
  5.10.68-1-MANJARO LLVM 12.0.1) 
  v: 4.6 Mesa 21.2.2 

[veracioux]

This is unfortunately the behavior of QSettings. But I have an idea on how to make this work so I'll look into it and let you know.

[mmahmoudian]

@veracioux Should I mark this as "bug" or "enhancement"?

[veracioux]

@mmahmoudian Definitely "enhancement" since INI files use ; for comments by default.

[borgmanJeremy]

@veracioux How do you feel about getting rid of QSettings and using a 3rd party TOML library? Not saying you have to be the one to do the work, just curious what your thoughts are on the merits of it.

[mmahmoudian]

INI files use ; for comments by default.

You are absolutely right, I mistakenly thought the # is the comment, although I have tested and it seems the lines starting with # (or %23) are ignored by Flameshot. Nonetheless, I belive the config file should change from ASCII to hex code if it is not necessary.

For those who are like me and forgot the specs of INI: https://en.wikipedia.org/wiki/INI_file#Comments

@borgmanJeremy I'm probably not as technical as you and other devs, but I just want to point out that if we move to TOML (or any other format for that matter), we should somehow mitigate migration issues users might face. Perhaps for few versions, we should have a part in the code to convert INI to TOML and inform users about the migration.

Regarding the format, I also agree with TOML as it is intuitive to understand, has good documentation, and is widely used (increases the chance that the user has already worked with it).

[veracioux]

@borgmanJeremy I'm working on custom read and write functions for QSettings which should solve this issue.

I don't think we should switch to TOML. TOML is very similar to INI and flameshot's config is not very complex, so I don't see any advantage. And I hate to involve 3rd party libraries unless necessary.

[kpcee]

I don't understand the sense of this suggestion. You can't comment out rows in QSettings, lines with a starting ";" are ignored and this has the same effect as deleting the whole line. QSettings uses name-value pairs (QMap<QString, QVariant>). If you change the name you create a new name-value pair which the program loads but cannot process internally. The next call of the original key would only create a new one with the given default value and you would have a duplication in the ini file.

Is this intentional or should the entire configuration be iterated for each entry to see if there is a identical entry but with a special character (#;) at the beginning in order to not save the name-value pair?

This is a very big effort for a very small benefit. It would be better to insert a fake comment at the top that points out the problem e.g. with

settings.beginGroup("Do not edit this file manually! WARNING!");
settings.endGroup();

QSettings certainly has its quirks, but it is used by thousands of programs and I have never heard of users having problems with it.

[veracioux]

I agree with @kpcee. I tried to make # a comment character, but I hit way too many roadblocks. It's not worth it. I would change the fake comment to "WARNING: Comments will be automatically deleted and lines may be automatically reordered". It can still be very useful to edit the file manually, provided you know what you are doing.

[borgmanJeremy]

Im good w/ that too.

[mmahmoudian]

@kpcee the original post stemmed from a mistake of mine that I thought in INI files lines leading with # are comments.

Personally I believe comments and order of lines are important and if QSettings cannot properly handle it, regardless or being used by the whole world or a single application, it has serious shortcomings.

---

@veracioux & @borgmanJeremy

All that said, regardless of what I believe to be "correct", I acknowledge that I'm not C++ savvy enough to know what are the implications of manually mending this bad behaviour, so I'll be fine with any solution that works and is easy to implement for you devs. Therefore, I'm fine with what you all are fine with 👍🏼