PunchThrough / node-red-contrib-bean

Node-Red nodes for the LightBlue Bean
23 stars 16 forks source link

Differentiating input/output nodes. #3

Closed garnold closed 9 years ago

garnold commented 9 years ago

... and removing outputs from "write scratch" node.

screenshot 2015-04-21 13 13 02

garnold commented 9 years ago

@raykamp (Bump)

garnold commented 9 years ago

@raykamp For completeness, I attempted to change the code to use <space>in and <space>out, and the Bean nodes disappeared from the palette. Perhaps -input and -output are to be used when the category contains spaces?

Either way, the version you merged is the right one. Thanks!

garnold commented 9 years ago

@raykamp Actually scratch that last comment. <space>in and <space>out apply to the node type, ie. "bean accel" or "bean led", whereas -input and -output apply to the node category, ie. "LightBlue Bean". The former is documented here; the latter is undocumented as far as I can tell.

Both conventions are used liberally and together throughout nodes included with the Node-RED, however it appears that the in/out convention does not play nicely with node types containing spaces, ie. "bean accel". I'll post a note to the list.

raykamp commented 9 years ago

Thank you for your explanation, @garnold. I'm planning to push an update to rename the original 4 nodes to use name like "Acceleration", "LED", "Serial", and "Temperature". I think it will better follow convention. The downside is that it's a small change that isn't backwards compatible. By upgrading, it will break existing flows that include those nodes. An input on this?

garnold commented 9 years ago

@raykamp No problem! So the only reason that I see to differentiate nodes using the <space>in and <space>out convention is when you have both an input and output version of the node, and want to use the same name in the UI, for example "http", "tcp", and "mqtt". Also note that often the output nodes appear in a different category in the palette from the input nodes.

In the case of the Bean nodes, the only nodes that conform to that rule are our new scratch nodes, which up to now have had different names ("read scratch" and "write scratch"). To that end, I've opened #4.

The other Bean nodes are fine as is unless you foresee creating complementary input/output nodes for each. Of course, if you are going to merge #4 then it might just make sense to rename all nodes now for consistency.

Also, it appears that I was wrong with the following assertion:

it appears that the in/out convention does not play nicely with node types containing spaces

The issue was actually that I didn't update all the references to the node types in the node HTML files.

garnold commented 9 years ago

@raykamp Also, if you do decide you would like to rename all nodes using the in/out convention, I'm happy to add that to #4 for you.

garnold commented 9 years ago

@raykamp Last note I promise :smile:

I just opened an issue regarding documenting the -input, -output convention.

knolleary commented 9 years ago

@raykamp rather than change the node types, which, as you rightly say will break all existing users, you could set the paletteLabel property to change how it appears in the palette without changing the underlying type.

raykamp commented 9 years ago

Thanks for the advice, @knolleary !