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

Preserve the order of non-grouped nested parameters #44

Merged

Conversation

sleiner
Copy link
Contributor

@sleiner sleiner commented Nov 12, 2022

This resolves issue #43. Nested parameters that are not grouped are now inserted to the parameter list according to definition order. Grouped parameters remain at the bottom of the parameter list.

@sleiner
Copy link
Contributor Author

sleiner commented Nov 12, 2022

@robbert-vdh seems like the Windows disk space issue is back 🙈

@robbert-vdh
Copy link
Owner

Haven't had any time yet, I'll take a look at this in a bit! I think if the order is preserved then it should probably do this for every parameter regardless of whether they're grouped or not so it's consistent. And that Windows disk space issue should be gone again with c92c2a1. I messed up the conditional.

@sleiner
Copy link
Contributor Author

sleiner commented Nov 13, 2022

I think if the order is preserved then it should probably do this for every parameter regardless of whether they're grouped or not so it's consistent.

I cannot make that fit together with your statements from #43:

This is indeed expected and intended behavior as this is the same order the parameters will appear in when the DAW uses a tree view (which in DAWs usually seem to display parameters above other subgroups).

non-grouped nested parameters (unlike the ones from the example above) would not have to be moved to the bottom, right?

They would not, no. This is just how the macro is implemented now.

So, I guess I was misunderstanding you either here or there. 😅 Could you explain your approach again?


And that Windows disk space issue should be gone again with c92c2a1.

Very nice, I rebased the branch to the latest master 👍🏻

@robbert-vdh robbert-vdh merged commit 6036db4 into robbert-vdh:master Nov 17, 2022
@robbert-vdh
Copy link
Owner

Sorry for taking this long to merge this! Thanks a lot for all the work, and especially also for adding tests! I fixed the order for grouped parameters, added some more tests to also cover grouped grouped parameters and to check whether the groups are handled correctly, and fixed the persistent fields for nested but not grouped parameters: 6036db4...2cddd70.

So, I guess I was misunderstanding you either here or there. sweat_smile Could you explain your approach again?

Listing non-nested fields first and nested fields seconds mimics one of the two common ways to show tries (directories before files, or directories after files). In the case of parameters having the flat list be in the order of the declarations also makes sense. But a halfway approach where some nested parameters (those without group attributes) are use the declaration order and other nested parameters (those with a group set) are listed last feels even more inconsistent to me. So my preference is to either follow the declaration order exactly or to do the individual-params-before-nested-params thing, but not something in between.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants