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

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

Closed
rcoreilly opened this issue Jan 14, 2024 · 14 comments
Assignees
Labels
bug Something isn't working correctly
Milestone

Comments

@rcoreilly
Copy link
Member

maybe when you change a setting? not clear.

@rcoreilly rcoreilly added this to the v2 milestone Jan 14, 2024
@rcoreilly rcoreilly added the bug Something isn't working correctly label Jan 14, 2024
@rcoreilly
Copy link
Member Author

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

@kkoreilly
Copy link
Member

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

@kkoreilly kkoreilly reopened this Jan 17, 2024
@kkoreilly
Copy link
Member

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
Copy link
Member

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

@rcoreilly
Copy link
Member Author

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
Copy link
Member

I added a unit test to tomls: it is definitely not just adding: see 288f7e9.

@rcoreilly
Copy link
Member Author

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

@rcoreilly
Copy link
Member Author

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
Copy link
Member

Should we file this as an issue upstream?

@rcoreilly
Copy link
Member Author

no I think it is intrinsic to the format.

@kkoreilly
Copy link
Member

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
Copy link
Member Author

rcoreilly commented Jan 17, 2024

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?

@rcoreilly rcoreilly changed the title FavPaths are replicating somehow submit an issue for toml package to reset slices instead of always adding to them (for slice fields, e.g., FavPaths) Feb 5, 2024
@kkoreilly
Copy link
Member

I filed the issue (pelletier/go-toml#931).

@kkoreilly
Copy link
Member

The issue has been resolved (see pelletier/go-toml#934 and 17911aa).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working correctly
Projects
None yet
Development

No branches or pull requests

2 participants