I've been trying to upgrade our PS version and have some problems to report. This email concerns issues with the changes to the printer in commit 60154a. Accordingly, the code samples below need to be formatted in a fixed-width font to be readable. Also, they're all in JS rather than PS for obvious reasons. I can provide the corresponding PS forms if desired.
First, a simple for loop used to be printed like this:
for (var n = 0; n < 10; n += 1) { blah(n); };
is now:
for (var n = 0; n < 10; n += 1) { blah(n); };
Not sure if that was intended, but it's so nonstandard that I can't bear to look at it. If one were going to put line breaks in the for loop declaration, surely there ought to be a line per clause rather than an arbitrary break between the second and third. But three lines is too many and anyway the whole thing is weird; it really belongs on one line.
Second, there seem to be bugs in how expressions in a comma-delimited list are line-separated and indented. Apologies for the contrived JS here, but I have to make the lines long enough to trigger an indentation. Code that used to be in one line like this:
var obj = (obj3433 = { }, (obj3433.foooooooooooo = 'oooooooooooooooooooooooooooooooo', obj3433.baaaaaaaaar = 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa', obj3433));
is now being indented like this:
var obj = (obj3433 = { }, (obj3433.foooooooooooo = 'oooooooooooooooooooooooooooooooo', obj3433.baaaaaaaaar = 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa', obj3433));
I like the idea, but it seems the third (and any subsequent) lines ought to be aligned with the second.
Here's a real example of the same problem from our code:
var bucket = (g4 = m1 - sol[label], (g5 = m2 - sol[label], (g6 = rcplus === 'col' ? rect3437[0] : rect3437[2], (g7 = rcplus === 'col' ? rect3437[1] : rect3437[3], rc === 'col' ? [g4, g5, g6, g7] : [g6, g7, g4, g5]))));
There are other more complex examples of nested grouped expressions that are coming out pretty strangely:
var sh = ( obj3439 = { markup : packIgrid(markup), tiling : csh.tiling, ultc : ultc, ultr : ultr }, ( (g3 = (it = csh.coldeltas, it != null && it !== false ? xcopy(it) : null), g3 != null ? (obj3439.coldeltas = g3) : null), ( g4 = (it3440 = csh.rowdeltas, it3440 != null && it3440 !== false ? xcopy(it3440) : null), g4 != null ? (obj3439.rowdeltas = g4) : null), obj3439));
It does seem like a good idea to not have all this on one line, but it's far from clear what the indentation policy is.
Similar inconsistencies afflict curly braces:
queueMsg(brl, { acknowledge : true }, mailbox);
or:
return ss.appointmentTicket = { col : c + (cols || 0), row : r + (rows || 0) };
and here's a monster one:
var REGRESSIONS = [{ type : 'js', target : 'ff', file1 : 'f1', file2 : 'f2' }, {
type : 'js',
target : 'ie6',
file1 : 'ie61',
file2 : 'ie62' }, {
type : 'js',
target : 'ie7',
file1 : 'ie71',
file2 : 'ie72' }, {
type : 'js',
target : 'ie8',
file1 : 'ie81',
file2 : 'ie82' }, {
type : 'js',
target : 'safari',
file1 : 'saf1',
file2 : 'saf2' }, {
type : 'js',
target : 'chrome',
file1 : 'chr1',
file2 : 'chr2' }, {
type : 'js',
target : 'node',
file1 : 'n1',
file2 : 'n2' }, {
type : 'html',
target : 'ff',
file1 : 'hf1',
file2 : 'hf2' }, {
type : 'html',
target : 'ie6',
file1 : 'hie61',
file2 : 'hie62' }, {
type : 'html',
target : 'ie7',
file1 : 'hie71',
file2 : 'hie72' }, {
type : 'html',
target : 'ie8',
file1 : 'hie81',
file2 : 'hie82' }, {
type : 'html',
target : 'safari',
file1 : 'hsaf1',
file2 : 'hsaf2' }, {
type : 'html',
target : 'chrome',
file1 : 'hchr1',
file2 : 'hchr2' }, {
type : 'css',
target : 'chrome',
file1 : 'c1',
file2 : 'c2' }, {
type : 'css',
target : 'ie8',
file1 : 'ie1',
file2 : 'ie2' }];
I have some respect for how much harder this problem is than it appears to be, having worked on similar issues for our Numen REPL and not being too happy with what I came up with. That being said, these changes to the printer probably make the JS less readable in our case rather than more readable. It would be better to do less and have it be consistent. At a minimum, I hope we can fix the more indecorous areas.
Daniel