the-graph
the-graph copied to clipboard
TheGraph.findMinMax() assumes metadata is numerical but they could be strings
I'm using loadFBP() to load a fbp file a bit like the following :
node1(bla1:label=bla1,x=36,y=36,width=72,height=72) OUT1 -> IN1 node2(bla2:label=bla2,x=144,y=36,width=72,height=72)
Unfortunately, the x,y, width, and heights are interpreted as strings, eg :
{
'x': '36',
'y': '36',
'width': '72',
'height': 72'
}
I'm not sure how the fbp parser can determine if they should be strings or not, but TheGraph.findMinMax() adds two of these values together and because they're strings the result is huge.
https://github.com/the-grid/the-graph/blob/master/the-graph/the-graph.js#L110
Perhaps it would be appropriate to put parseInt() around those values?
I'm happy to make a PR...
@bergie should we parseInt for certain metadata in fbp? And/or be safe here?
@davidmaxwaterman could do the paresInt() thing within findMinMax, but maybe it wouldn't hurt to do a cleanup pass before https://github.com/the-grid/the-graph/blob/9c5e6d6715de8a8de897a82f62228fcce286e4b8/the-graph-editor/the-graph-editor.html#L269
IMO, the values should be integers as soon as they're read (this sort of error could happen anywhere, no just in findMinMax()), and the fix should go into fbp parser...surely, something can be added to :
https://github.com/noflo/fbp/blob/master/grammar/fbp.peg
to convert them if they're values that are known to be numeric? Somewhere around Line 32, I expect :
https://github.com/noflo/fbp/blob/master/grammar/fbp.peg#L32
I might have a go at getting that to work...
I should add that there is also an issue with the fbp parser where it lower-cases the port names, and that causes the ports to be duplicated. Why would it lower-case them, I wonder? I've added code to go through them and uppercase them again and it works as expected? Comments?