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

MyDeveloperDay via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Jan 6 01:25:45 PST 2022


MyDeveloperDay added a comment.

I would say for the `ColumnLimit:0` case, we don't have to wrap a single import like this: for `JavaScriptWrapImports :true`

  import {
     Get
  }  from '@nestjs/common';

For more than one import then I'd say it should do:

  import {
     Get,
     Req
  }  from '@nestjs/common';

What tells me that is the "s" at the end of "Imports" of  `JavaScriptWrapImports` meaning more than one. i.e. we don't wrap on 1 but we do on 2 and above

For  `ColumnLimit: 0`  and `JavaScriptWrapImports: false`  then I think that means we shouldn't touch the imports at all.

This is becuase in this case we don't really want "true/false" we want "Leave/Always/Never", I don't think we should assume `false` means `Never` and hence its time to disambiguate.

This is part of our natural lifecycle of all options: (which tend to follow this patter)

1. they start as booleans
2. they become enums
3. they become structs

It probably means we've got to the point where they should be changed to an enum so we don't have to guess what all our 100,000's of users will want but give them the power to do what they need.


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

https://reviews.llvm.org/D116638



More information about the cfe-commits mailing list