[PATCH] D116638: [clang-format] Fix ignoring JavaScriptWrapImport when ColumnWidth: 0

[Andrey Mishchenko via Phabricator via cfe-commits]

andmis added a comment.

Thanks for the feedback. Two things:

1. Force-breaking at >= 2 imports and not breaking at 1 import feels has the advantage of being simple to state, implement, document, and test, but I don't think it's actually the behavior people will want. For example, the original bug reporter apparently thought about it too and apparently came up with the same semantics I tried to implement as his idea of "the right thing": https://github.com/llvm/llvm-project/issues/52935.
2. The issue isn't whether we use an enum or a boolean. The issue is that it is not clear what the semantics should be of "keep input file's breaking decisions". If `SortIncludes: true`, what should "keep" do here?

  import {
    A,
    B, C,
  } from 'url';
  
  import {
    A,
    C, B,
  } from 'url';

In my opinion, this could just be merged, pending a larger revisit: (1) very few people use `ColumnLimit: 0`, at least with JS code, else we would hear more complaints, since import wrapping is currently really not good in this case, (2) this patch applies the correct fix in the case `ColumnLimit: 0`, `JavaScriptWrapImports: false`, (3) this patch admittedly does not completely solve `ColumnLimit: 0`, `JavaScriptWrapImports: true`, but is not a regression there, either.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116638/new/

https://reviews.llvm.org/D116638