Make it a runtime error to call `Debug.watch` on a `Signal`?

[jvoigtlaender]

Same for Debug.watchSummary.

Reason: Calling Debug.watch on a Signal is never what you want. But it can easily happen accidentally. It would be helpful if the debugger errored out with a useful error message in such a case, instead of simply ignoring the Debug.watch call and leaving you to wonder why no watch is being shown.

See this mailing list message and this related PR: #264.

[TheSeamau5]

Perhaps a warning should be best?

I mean, a signal is an immutable value. Sure, it represents something that mutates with time but it's just a value. It's like watching a function, you get something that is not very useful.

[TheSeamau5]

Perhaps a warning should be best?

I mean, a signal is an immutable value. Sure, it represents something that mutates with time but it's just a value. It's like watching a function, you get something that is not very useful.

[jvoigtlaender]

Yes, it could be a warning. Though I don't really see the point of not making it as strong as an error. After all, the whole Debug module is explicitly not for production use. You only use it during development. At that time, during development, why not simply tell the programmer "That call of Debug.watch there is completely meaningless. I'm not even going to run your program, since clearly this is not what you want (adding a watch that will never show anything)."

Also, how exactly does a "warning" surface inside elm-reactor? Because that is where you will be looking at your program during debugging. Not on a command line where a compiler warning might appear. (Also, note that one could make it so that the runtime error only happens if one uses elm-reactor in debug mode, not in normal mode. Already now it is the case that Debug.watch only does anything at all if the program is running in debug mode.)

[jvoigtlaender]

Yes, it could be a warning. Though I don't really see the point of not making it as strong as an error. After all, the whole Debug module is explicitly not for production use. You only use it during development. At that time, during development, why not simply tell the programmer "That call of Debug.watch there is completely meaningless. I'm not even going to run your program, since clearly this is not what you want (adding a watch that will never show anything)."

Also, how exactly does a "warning" surface inside elm-reactor? Because that is where you will be looking at your program during debugging. Not on a command line where a compiler warning might appear. (Also, note that one could make it so that the runtime error only happens if one uses elm-reactor in debug mode, not in normal mode. Already now it is the case that Debug.watch only does anything at all if the program is running in debug mode.)

[jvoigtlaender]

Actually, I retract my addition "Same for Debug.watchSummary." It's hard enough even to accidentally write a type-correct application of Debug.watchSummary where the third argument is a signal (because of the type of the second argument). So there's no need to "catch" this at runtime.

But for Debug.watch the issue is real, as evidenced again here.

[jvoigtlaender]

Actually, I retract my addition "Same for Debug.watchSummary." It's hard enough even to accidentally write a type-correct application of Debug.watchSummary where the third argument is a signal (because of the type of the second argument). So there's no need to "catch" this at runtime.

But for Debug.watch the issue is real, as evidenced again here.

[evancz]

I think @vilterp can say more about whether this is a good idea. There are some changes to visualization coming in elm-reactor and the level of integration with the Debug module may shift a bit.

I feel like causing a crash is not the right move here though.

[evancz]

I think @vilterp can say more about whether this is a good idea. There are some changes to visualization coming in elm-reactor and the level of integration with the Debug module may shift a bit.

I feel like causing a crash is not the right move here though.

[vilterp]

I think it's possible to have this just work (i.e. you see a watch/log in the sidebar), since the infrastructure for the debugger to subscribe to signals and get updates about them is now in place. Two issues come to mind:

1. it seems like the log/watch distinction should go away — the sidebar can show logs, and watches are just logs in a collapsed state (see this issue for details / screenshots). So, it seems like there only needs to be one function. Does Debug.watch or Debug.log make more sense?
2. Accurately logging updates: when showing a signal's log, there should only be an entry each time the node actually updates (i.e. when the parentUpdate argument to that signal's notify function is true, and it actually recomputes its value). It seems we need to add an "updated this frame" flag to signal graph nodes to make this work.

Aside from that, I would just have to make a special case for signals in Debug.watch, and modify the log-showing code a bit. I think this would be a cool feature!

[vilterp]

I think it's possible to have this just work (i.e. you see a watch/log in the sidebar), since the infrastructure for the debugger to subscribe to signals and get updates about them is now in place. Two issues come to mind:

1. it seems like the log/watch distinction should go away — the sidebar can show logs, and watches are just logs in a collapsed state (see this issue for details / screenshots). So, it seems like there only needs to be one function. Does Debug.watch or Debug.log make more sense?
2. Accurately logging updates: when showing a signal's log, there should only be an entry each time the node actually updates (i.e. when the parentUpdate argument to that signal's notify function is true, and it actually recomputes its value). It seems we need to add an "updated this frame" flag to signal graph nodes to make this work.

Aside from that, I would just have to make a special case for signals in Debug.watch, and modify the log-showing code a bit. I think this would be a cool feature!

[jvoigtlaender]

Of course, having it just work, so that this note becomes obsolete, would be much better than a runtime error. 😄

[jvoigtlaender]

Of course, having it just work, so that this note becomes obsolete, would be much better than a runtime error. 😄

[evancz]

Alright, I am going to close this. I think we can do better than a runtime error, so it does not make sense to make it weird in the meantime.

[evancz]

Alright, I am going to close this. I think we can do better than a runtime error, so it does not make sense to make it weird in the meantime.