getNewick() returns unquoted Newick strings

[gaurav]

I've been trying to use tree.getNewick() to implement a custom menu item in phylotree.js v1.4.0 that can be used to rename a node. The algorithm I use is:

1. The custom menu item is added to every node in the phylogeny, and asks the user for a new name for that node.
2. It then sets that node's node.name property to the new string, surrounding it in double-quotes in case the user has included whitespace. This updates the internal Newick representation of the phylogeny.
3. It then uses tree.getNewick() to retrieve the complete Newick representation of the phylogeny, and redraws the tree with the new Newick string.

This seems to work well for Newick strings that don't have any whitespace. However, when my Newick string does have whitespace, tree.getNewick() returns a string like:

(Tomistoma schlegelii:1,(Osteolaemus tetraspis:1,Crocodylus niloticus:1,"Test test":1)Crocodylinae:1);

i.e. only the name that I've explicitly quoted has quote characters, and other names -- even those containing spaces -- are loaded unquoted. This is incorrect Newick, which requires quoting with single quoting for names containing strings (see Wikipedia and the Newick spec). When loading this new Newick string, phylotree.js (correctly) ignores spaces, so "Crocodylus niloticus" is rendered as Crocodylusniloticus.

I think the issue is in the following line of code, which doesn't quote names as they are exported:

phylotree.js/src/formats/newick.js

Line 253 in ca174f3

element_array.push(n.data.name);

Could you please modify this line of code so that names being exported are properly quoted? As per the spec, names should be quoted with single quotes before and after the name, and any single quotes present within the name should be doubled to escape it (i.e. ab'de should be written as 'ab''de')

[gaurav]

I took a stab at fixing this in PR https://github.com/veg/phylotree.js/pull/441/files -- please let me know if this is useful for you, and if you have any suggestions for improvement.

[gaurav]

Thanks so much for improving and merging PR #441! Do you know when there might be a release containing that change? I tried using phylotree.js directly from GitHub (using npm install --save https://github.com/veg/phylotree.js), but that doesn't seem to include the CSS files, so I don't think I can test this issue until I can install phylotree from npm.

[stevenweaver]

Thanks @gaurav for your contribution. I've released v1.4.1 which should include changes from #441.

[gaurav]

Thank you, Steven! This upgrade has fixed most of our node renaming issues. There's still a minor bug when renaming a node to a name containing a single quote (see phyloref/klados#312); I'll continue poking at it and see if it's being caused in my software or in phylotree.js.