Allow node name to be auto-generated when added

[knolleary]

• New feature (non-breaking change which adds functionality)

Closes #3418

Proposed changes

The original proposal has been replaced by an action based approach

Adds a new action:

• core:generate-node-names

When invoked, will apply a default name to the selected nodes.

• It only applies to nodes that do not currently have a name
• It uses a name of the form <paletteLabel> <N> where N is the next available integer that avoids a name clash. This is scoped to the whole editor - not just individual flows.

It can also be invoked on an individual node to allow it to be used in a node's onadd function to generate a name when being added to the workspace.

This PR adds onadd functions to the debug and link in/out/call nodes - as they all most benefit from having more human readable, distinct, default names.

[coveralls]

Coverage remained the same at 67.323% when pulling 3c0b740 on auto-gen-name into 6a5c50f on dev.

[Steve-Mcl]

Hi nick. This might be feature creep but I think 2 more features would make this even sweeter...

1. An action to run the auto making against selected nodes (would make quick work of updating an existing flow)
   1. Perhaps a linting corrective action could also call this feature?
2. Make the auto naming work with split wires to links.

[knolleary]

@Steve-Mcl you're quite right an action is the way to go.

Having thought about it some more, I'm inclined to completely change the approach and remove the autoGenerate property from the defaults object - and go back to this being something a node can do via an onadd function by invoking the new action.

[knolleary]

The consequence of moving to an Action approach is we no longer allows individual nodes to customise the behaviour:

• we use <paletteLabel> <N> as the name format. It cannot be translated unless a node provides its own paletteLabel function to return a translation
• the 'scope' of the number is fixed a single behaviour. It can either be scoped to the flow, or globally - we can't have some nodes doing one thing and others another.

In the live-stream where I put this together, a lot of people wanted 'flow' scoped number for the Debug nodes. That's a reasonable choice for that node type, but it wouldn't work (I don't believe) for Link nodes - which are more likely to want to reference each other across flows.

On balance, I think going with globally scoped numbering is the best option.

[knolleary]

I have replaced the implementation with an action based approach and updated the PR description to match

[Steve-Mcl]

Nick, I've been running DEV with this change merged. I'm liking it :+1

That said, there is one frustruation.

• I drop a new debug it gets default "debug 1" name.
• I setup that node to my liking
• I then copy/paste it (as it has all the settings I want) but the name remains "debug 1"!

Would you consider applying the auto name generation if a nodes name meets the pattern '^'+paletteLabel+' (\\d+)$' ?

Alternatively, we could set an autogenName property to same value as the name property & if name == null || autogenName === name then run the auto naming?

[knolleary]

Would you consider applying the auto name generation if a nodes name meets the pattern '^'+paletteLabel+' (\d+)$' ?

Currently the copy/paste handling is the same as the import flow handling - and we wouldn't want to start renaming nodes in flows that are imported.

That said, the import function does take some flags to customise its behaviour - such as whether to generate new Ids or not. So it would be feasible to add a flag to cause naming updates for paste actions only.

[Steve-Mcl]

That said, the import function does take some flags to customise its behaviour - such as whether to generate new Ids or not. So it would be feasible to add a flag to cause naming updates for paste actions only

I think the experience would be greatly improved if we did Nick.

Are you ok with me raising an issue around this so its tracked?

[dceejay]

Does this also force new names into "old" nodes that have no name ? While I may want to name debug nodes I also like being able to see that it is set to msg.payload or msg.foobar or just msg etc

[Steve-Mcl]

Does this also force new names into "old" nodes that have no name

No Dave, only newly added nodes & imported/pasted nodes without a name.

It does mean however to see msg or msg.xxxx as the name on a newly added debug node, you would need to open the editor & clear out the auto name - BUT - you would need to open the editor anyway (to set the debug node to something other than msg.payload).

[knolleary]

are you ok with me raising an issue around this so its tracked?

Yes please.

Does this also force new names into "old" nodes that have no name
No Dave, only newly added nodes & imported/pasted nodes without a name.

The 'rename on paste' should only happen if the existing name matches the auto-generated name pattern. It should not modify nodes with no name.

[dceejay]

With the current version in dev - If I have an existing node with no name - (showing msg.payload as label) - and copy/paste it to create another - it becomes "debug 1" - If I then copy paste that is creates another "debug 1". I would rather those actions were swapped... if blank - leave blank... if debug 1 then become debug 2... etc

[knolleary]

and copy/paste it to create another - it becomes "debug 1"

I may have lost track, but I don't think that is intended behaviour.

I agree the expected behaviour should be to not change names unless they match the autogenerated name pattern.

[dceejay]

This is existing dev branch... not sure if @Steve-Mcl is still working on this branch.

[knolleary]

Nothing has been changed in dev since this PR - so that is a bug with this code that should be resolved as part of the improving copy and paste as per the discussion above.