Add loop parameter names #65
Conversation
|
I see what the intent with the PR is but I am not a fan of the execution. I think we should ask the others that are working on the debugger cc @LastTalon @metrowaii
|
|
I think we want to make the debugger more modular and maybe later down the line we can address this. I think we should close this and maybe revive it until then. |
|
You're welcome to suggest a different implementation, but the debugger being pulled out into a module or not wouldn't really have an effect on this PR. |
|
We can potentially revisit this interface later if we do more debugger reworking. For now, however, this is the interface that I think is the least intrusive and still has the desired effect. The nearest alternative that has the least intrusion is to set the And in reality, this is not meant to display what the stringified object is, but the label we choose to give it. E.g. if we pass an actual string as a loop parameter, we don't really want the debugger to display the string in this place, we want a label for what the user calls that parameter. So these are actually different things. Thus, as far as I can tell we must provide an additional string here because the semantics of that string are for what the user wants to label their parameters. And it has to be given to the debugger because the loop doesn't care what the user calls them, only the debugger does. Feel free to leave additional comments. Jack and I worked on the interface for this together and this is what we settled on as a minimal implementation that hits all of those points. |
|
I am unsure of a better implementation but keeping a separate table of strings makes it more onerous as to which label corresponds to which object imo. I understand the problem that this has to be done at the debugger. Which is why i propose an orthogonal api that would be something like this
After the arguments have been added and the debugger is built, we add the arguments in order to create the loop class. I think this makes the expression more intentful with the labels. However the drawback is very clearly that to get the loop instance you have to get it from the debugger which feels like it has a pretty backwards relation. Not sure if there is a better way to do this that keeps a flat topology. |
|
I don't think this is a good choice of API. The debugger shouldn't be creating a loop. If you think the parameters need to be associated, I understand that argument and in that case we should pass a map. But I don't really think I agree, again, because of the way the parameters are given, not by value, but by position. If we look at proposed API in usage this is very clear and even symmetrical. |
Adds an extra field to the Debugger called
loopParameterNamesto specify the names you want displayed in the Matter debugger, rather than the defaultWorld 1,table 2,table 3, etc.