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

[Stanisław Małolepszy via Phabricator via cfe-commits]

stasm added a comment.

In D116638#3547366 , @andmis wrote:

> In D116638#3545246 , @stasm wrote:
>
>> I'm still interested in seeing this fixed. Would it help if I rebased this 
>> change and addressed the outstanding review comments?
>
> Go for it! I don't plan to do any further work on this. (I'm the original 
> author of the patch.)

Thanks for letting me know! I'll see what I can do :)


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

https://reviews.llvm.org/D116638

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Stanisław Małolepszy via Phabricator via cfe-commits]

stasm added a comment.
Herald added a project: All.

I'm still interested in seeing this fixed. Would it help if I rebased this 
change and addressed the outstanding review comments?


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

https://reviews.llvm.org/D116638

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Stanisław Małolepszy via Phabricator via cfe-commits]

stasm added a comment.

The reporter of issue 52935  
here. Thanks, @andmis, for your work.  Thinking about the `ColumnLimit: 0` and 
`JavaScriptWrapImports: false` case, it seems that there are two issues in the 
current implementation that could be solved separately.

1. Single-line imports get force-wrapped despite `JavaScriptWrapImports: 
false`. This seems to be a clear bug in `clang-format`. The expected behavior 
in this case should be to not touch the import line at all. Instead, the 
current behavior is the following:

  import {aaa, bbb, ccc} from "def";



  import {aaa,
  bbb,
  ccc} from "def";



2. It's not clear what `JavaScriptWrapImports: false` should do to multiline 
imports when `ColumnLimit: 0`. Should it
  - force-unwrap to a single line, or
  - leave the import as-is (i.e. //not force-wrap// it)?

Since the expected behavior is not clear there might indeed be different 
groups of users expecting one behavior or the other. To reduce the ambiguity an 
enum option like the one proposed by @MyDeveloperDay  would be helpful.

I'd personally would love to see both of these issues addressed (and I'd be a 
happy user of `JavaScriptWrapImports: Never` if it's available), but just 
fixing the first bug would go a long way in making `ColumnLimit: 0` a viable 
setting for JavaScript for me.


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

https://reviews.llvm.org/D116638

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits