[PATCH] D66584: [clang-format] Support .mjs module javascript extension in clang-format

2019-08-22 Thread Fangrui Song via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL369732: [clang-format] Recognize ECMAScript module .mjs as 
JavaScript (authored by MaskRay, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D66584?vs=216588=216766#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66584

Files:
  cfe/trunk/lib/Format/Format.cpp


Index: cfe/trunk/lib/Format/Format.cpp
===
--- cfe/trunk/lib/Format/Format.cpp
+++ cfe/trunk/lib/Format/Format.cpp
@@ -2411,8 +2411,9 @@
 static FormatStyle::LanguageKind getLanguageByFileName(StringRef FileName) {
   if (FileName.endswith(".java"))
 return FormatStyle::LK_Java;
-  if (FileName.endswith_lower(".js") || FileName.endswith_lower(".ts"))
-return FormatStyle::LK_JavaScript; // JavaScript or TypeScript.
+  if (FileName.endswith_lower(".js") || FileName.endswith_lower(".mjs") ||
+  FileName.endswith_lower(".ts"))
+return FormatStyle::LK_JavaScript; // (module) JavaScript or TypeScript.
   if (FileName.endswith(".m") || FileName.endswith(".mm"))
 return FormatStyle::LK_ObjC;
   if (FileName.endswith_lower(".proto") ||


Index: cfe/trunk/lib/Format/Format.cpp
===
--- cfe/trunk/lib/Format/Format.cpp
+++ cfe/trunk/lib/Format/Format.cpp
@@ -2411,8 +2411,9 @@
 static FormatStyle::LanguageKind getLanguageByFileName(StringRef FileName) {
   if (FileName.endswith(".java"))
 return FormatStyle::LK_Java;
-  if (FileName.endswith_lower(".js") || FileName.endswith_lower(".ts"))
-return FormatStyle::LK_JavaScript; // JavaScript or TypeScript.
+  if (FileName.endswith_lower(".js") || FileName.endswith_lower(".mjs") ||
+  FileName.endswith_lower(".ts"))
+return FormatStyle::LK_JavaScript; // (module) JavaScript or TypeScript.
   if (FileName.endswith(".m") || FileName.endswith(".mm"))
 return FormatStyle::LK_ObjC;
   if (FileName.endswith_lower(".proto") ||
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D66584: [clang-format] Support .mjs module javascript extension in clang-format

2019-08-22 Thread Fergal Daly via Phabricator via cfe-commits
fergald added a comment.

Thanks, I'm happy with that description


Repository:
  rC Clang

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

https://reviews.llvm.org/D66584



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


[PATCH] D66584: [clang-format] Support .mjs module javascript extension in clang-format

2019-08-22 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision.
MaskRay added a comment.

Reading some materials online... I think this should be named ECMAScript module 
(ECM), not JavaScript modules.. I can fix the description and commit for you.


Repository:
  rC Clang

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

https://reviews.llvm.org/D66584



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


[PATCH] D66584: [clang-format] Support .mjs module javascript extension in clang-format

2019-08-22 Thread Fergal Daly via Phabricator via cfe-commits
fergald added a comment.

"why .mjs is so common that it justifies a clang-format change?"

I don't have data but Module JS is the future, it's supported by all major 
browsers for some time and also Node.js, it solves the problem of everything 
being lumped into one namespace.

Why does it need a different suffix? It adds the "export" and "import" keywords 
and so it incompatible with common JS. Forcing everyone to keep using js for 
both is not helpful (clang-format is not forcing of course but applying 
clang-format with --assume-filename is pretty awkward).

Node specifies .mjs as the suffix

https://medium.com/@nodejs/announcing-a-new-experimental-modules-1be8d2d6c2ff
https://nodejs.org/api/esm.html

So the use of the .mjs suffix is going to grow in volume as people modernize 
their project. It would be great for clang-format to be enabling that migration.


Repository:
  rC Clang

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

https://reviews.llvm.org/D66584



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


[PATCH] D66584: [clang-format] Support .mjs module javascript extension in clang-format

2019-08-22 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment.

> This causes .mjs files to be treated as javascript.

I think you meant .mjs was not recognized before but will be recognized as 
JavaScript with this change.

Can you elaborate a bit why .mjs is so common that it justifies a clang-format 
change?


Repository:
  rC Clang

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

https://reviews.llvm.org/D66584



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


[PATCH] D66584: [clang-format] Support .mjs module javascript extension in clang-format

2019-08-22 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment.

> module-javascript. Module javascript is not compatible with non-module java 
> script as it uses the "export" and "import" keywords which are only legal in 
> module-javascript.

java script -> JavaScript
module-javascript -> Module JavaScript?


Repository:
  rC Clang

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

https://reviews.llvm.org/D66584



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


[PATCH] D66584: [clang-format] Support .mjs module javascript extension in clang-format

2019-08-22 Thread Fergal Daly via Phabricator via cfe-commits
fergald added a comment.

I cannot commit this, please do so, if you're happy with it.


Repository:
  rC Clang

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

https://reviews.llvm.org/D66584



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


[PATCH] D66584: [clang-format] Support .mjs module javascript extension in clang-format

2019-08-22 Thread Fergal Daly via Phabricator via cfe-commits
fergald added a comment.

It seems to do the right thing. E.g.

https://crrev.com/c/1767284/5/third_party/blink/renderer/core/script/resources/layered_api/elements/virtual-scroller/visibility-manager.mjs


Repository:
  rC Clang

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

https://reviews.llvm.org/D66584



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


[PATCH] D66584: [clang-format] Support .mjs module javascript extension in clang-format

2019-08-22 Thread Nico Weber via Phabricator via cfe-commits
thakis accepted this revision.
thakis added a comment.
This revision is now accepted and ready to land.

Does clang-format format import and export lines in mjs files correctly?


Repository:
  rC Clang

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

https://reviews.llvm.org/D66584



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


[PATCH] D66584: [clang-format] Support .mjs module javascript extension in clang-format

2019-08-22 Thread Fergal Daly via Phabricator via cfe-commits
fergald created this revision.
fergald added a reviewer: MaskRay.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

PR43085.

This causes .mjs files to be treated as javascript. .mjs is the extension for 
module-javascript. Module javascript is not compatible with non-module java 
script as it uses the "export" and "import" keywords which are only legal in 
module-javascript. As such it uses a different file extension.


Repository:
  rC Clang

https://reviews.llvm.org/D66584

Files:
  lib/Format/Format.cpp


Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -2411,8 +2411,9 @@
 static FormatStyle::LanguageKind getLanguageByFileName(StringRef FileName) {
   if (FileName.endswith(".java"))
 return FormatStyle::LK_Java;
-  if (FileName.endswith_lower(".js") || FileName.endswith_lower(".ts"))
-return FormatStyle::LK_JavaScript; // JavaScript or TypeScript.
+  if (FileName.endswith_lower(".js") || FileName.endswith_lower(".ts") ||
+  FileName.endswith_lower(".mjs"))
+return FormatStyle::LK_JavaScript; // (module) JavaScript or TypeScript.
   if (FileName.endswith(".m") || FileName.endswith(".mm"))
 return FormatStyle::LK_ObjC;
   if (FileName.endswith_lower(".proto") ||


Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -2411,8 +2411,9 @@
 static FormatStyle::LanguageKind getLanguageByFileName(StringRef FileName) {
   if (FileName.endswith(".java"))
 return FormatStyle::LK_Java;
-  if (FileName.endswith_lower(".js") || FileName.endswith_lower(".ts"))
-return FormatStyle::LK_JavaScript; // JavaScript or TypeScript.
+  if (FileName.endswith_lower(".js") || FileName.endswith_lower(".ts") ||
+  FileName.endswith_lower(".mjs"))
+return FormatStyle::LK_JavaScript; // (module) JavaScript or TypeScript.
   if (FileName.endswith(".m") || FileName.endswith(".mm"))
 return FormatStyle::LK_ObjC;
   if (FileName.endswith_lower(".proto") ||
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits