Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Alignment with multiple adjacent strings #188

Closed
alan-knight opened this issue Feb 25, 2015 · 3 comments
Closed

Alignment with multiple adjacent strings #188

alan-knight opened this issue Feb 25, 2015 · 3 comments

Comments

@alan-knight
Copy link

This doesn't seem right. I'd expect the two later literal strings to be indented relative to the named argument. Or better yet, all 3 of the adjacent strings on separate lines, indented relative to the named arguments.

  method() => Intl.message("This comes from a method",
       name: 'method', desc: 'This is a method with a '
       'long description which spans '
       'multiple lines.');
@alan-knight
Copy link
Author

Another example.

      testHtml('DOM clobbering of attributes with multiple nodes', validator,
          "<form onmouseover='alert(1)'><input name='attributes'>"
          "<input name='attributes'>", "");

It's very unclear from this formatting that the last two arguments are two strings, the first the concatenation of two long adjacent strings and the second one empty. I'd prefer to have the second string on its own line, but even then it would seem confusing to have

   testHtml('DOM clobbering of attributes with multiple nodes', validator,
        "<form onmouseover='alert(1)'><input name='attributes'>"
        "<input name='attributes'>", 
        "");

I think I'd rather have subsequent lines of the adjacent string be indented.

@natebosch
Copy link
Member

Aligning relative to a named argument only works well with short argument names. In the case of long argument names it can look really bad.

@munificent
Copy link
Member

In the forthcoming tall style, this code gets formatted as:

  method() => Intl.message(
    "This comes from a method",
    name: 'method',
    desc:
        'This is a method with a '
        'long description which spans '
        'multiple lines.',
  );

And:

      testHtml(
        'DOM clobbering of attributes with multiple nodes',
        validator,
        "<form onmouseover='alert(1)'><input name='attributes'>"
            "<input name='attributes'>",
        "",
      );

I think these both do a good job of making the argument lists clearer (obviously at the expense of more vertical height, hence the name "tall" style).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants