search

cogentcore / core

A free and open source framework for building powerful, fast, and elegant 2D and 3D apps that run on macOS, Windows, Linux, iOS, Android, and the Web with a single pure Go codebase, allowing you to Code Once, Run Everywhere.
http://cogentcore.org/core
BSD 3-Clause "New" or "Revised" License
1.33k stars 71 forks source link

submit an issue for toml package to reset slices instead of always adding to them (for slice fields, e.g., FavPaths) #803

Closed rcoreilly closed 5 months ago

rcoreilly commented 5 months ago

maybe when you change a setting? not clear.

rcoreilly commented 5 months ago

toml only adds to existing apparently. added a reset in SystemSettings Open method. d54055ad9

kkoreilly commented 5 months ago

Not a working solution. This removes all support for default fav paths, so, for example, I now have no fav paths.

kkoreilly commented 5 months ago

Given that the fav paths seem to get reset correctly after you restart the app, it doesn't seem like a toml loading problem.

kkoreilly commented 5 months ago

Is it possible that something is going wrong with the SetToDefaults method? Have you checked whether DefaultPaths is somehow getting modified?

rcoreilly commented 5 months ago

I pushed a soln: if empty after loading, set to defaults.

it is definitely just adding in toml -- I'm pretty sure I had this problem in other cases too... could do more research about that, but this should be workin gnow

kkoreilly commented 5 months ago

I added a unit test to tomls: it is definitely not just adding: see https://github.com/cogentcore/core/commit/288f7e9a9fbbeddadb0d4c298d5d78a2f975dc1a.

rcoreilly commented 5 months ago

it only is a problem for []struct not []string -- check out the updated test that shows the problem.

rcoreilly commented 5 months ago

they have different save formats -- struct is separate [[Slice]] elements and it is clear from the format that it would have a hard time knowing when to reset.

kkoreilly commented 5 months ago

Should we file this as an issue upstream?

rcoreilly commented 5 months ago

no I think it is intrinsic to the format.

kkoreilly commented 5 months ago

Regardless of how difficult it would be to fix, this is a violation of the standard go marshalling and umarshalling paradigm that people would expect, so there should at least be a disclaimer upstream.

rcoreilly commented 5 months ago

it would have to have some kind of global var during outer loop of loading to detect when the first of each such slice element was encountered, and reset then. or just reset at the start. you could google about this further though. anyway, do you want to make the test pass or should i?

kkoreilly commented 5 months ago

I filed the issue (https://github.com/pelletier/go-toml/issues/931).

kkoreilly commented 4 months ago

The issue has been resolved (see https://github.com/pelletier/go-toml/pull/934 and 17911aa).