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

chore: add plugins files to format script #127

Merged
merged 2 commits into from
Sep 1, 2020
Merged

chore: add plugins files to format script #127

merged 2 commits into from
Sep 1, 2020

Conversation

carloslancha
Copy link
Contributor

@carloslancha carloslancha commented Aug 28, 2020

Since we're adding custom plugins to this repo we need to format them, but CKEditor doesn't support some of the rules we were working with, so here I'm adding a specific prettier configuration for plugins and updating the format scripts to make it work.

@carloslancha carloslancha requested review from wincent and julien and removed request for wincent August 28, 2020 11:20
@julien
Copy link
Contributor

julien commented Aug 28, 2020

👍 LGTM

@wincent
Copy link
Contributor

wincent commented Aug 28, 2020

but CKEditor doesn't support some of the rules we were working with

So, main config is:

{
	"bracketSpacing": false,
	"singleQuote": true,
	"tabWidth": 4,
	"trailingComma": "es5",
	"useTabs": true
}

and plugin config is:

{
	"bracketSpacing": false,
	"quoteProps": "preserve",
	"singleQuote": true,
	"tabWidth": 4,
	"trailingComma": "none",
	"useTabs": true
}

@carloslancha: Is the thing you mean by "CKEditor doesn't support" the trailingComma setting? I don't see anything in the default that would cause problems (even IE can handle the "es5" trailing comma set-up).

@carloslancha
Copy link
Contributor Author

@wincent I added "quoteProps": "preserve" too. What I mean is that CKBuilder warns you about it, this is an example with the codemirror plugin formated with original config

ADVERTENCIA: en.js:3: WARNING - Parse error. IE8 (and below) will parse trailing commas in array and object literals incorrectly. If you are targeting newer versions of JS, set the appropriate language_in option.
	source: 'Source',
	                ^

ago 28, 2020 1:39:32 PM com.google.javascript.jscomp.LoggerErrorManager printSummary
ADVERTENCIA: 0 error(s), 1 warning(s)
ago 28, 2020 1:39:32 PM com.google.javascript.jscomp.LoggerErrorManager println
ADVERTENCIA: codemirrordialog.js:28: WARNING - Parse error. IE8 (and below) will parse trailing commas in array and object literals incorrectly. If you are targeting newer versions of JS, set the appropriate language_in option.
					mode: 'text/html',
					                 ^

ago 28, 2020 1:39:32 PM com.google.javascript.jscomp.LoggerErrorManager println
ADVERTENCIA: codemirrordialog.js:81: WARNING - Parse error. IE8 (and below) will parse trailing commas in array and object literals incorrectly. If you are targeting newer versions of JS, set the appropriate language_in option.
					) || 0,
					      ^

ago 28, 2020 1:39:32 PM com.google.javascript.jscomp.LoggerErrorManager println
ADVERTENCIA: codemirrordialog.js:91: WARNING - Keywords and reserved words are not allowed as unquoted property names in older versions of JavaScript. If you are targeting newer versions of JavaScript, set the appropriate language_in option.
				class: 'cke_wysiwyg_frame cke_reset',
				^

ago 28, 2020 1:39:32 PM com.google.javascript.jscomp.LoggerErrorManager println
ADVERTENCIA: codemirrordialog.js:92: WARNING - Parse error. IE8 (and below) will parse trailing commas in array and object literals incorrectly. If you are targeting newer versions of JS, set the appropriate language_in option.
				frameborder: 0,
				              ^

ago 28, 2020 1:39:32 PM com.google.javascript.jscomp.LoggerErrorManager println
ADVERTENCIA: codemirrordialog.js:97: WARNING - Parse error. IE8 (and below) will parse trailing commas in array and object literals incorrectly. If you are targeting newer versions of JS, set the appropriate language_in option.
				width: '99%',
				            ^

ago 28, 2020 1:39:32 PM com.google.javascript.jscomp.LoggerErrorManager println
ADVERTENCIA: codemirrordialog.js:163: WARNING - Parse error. IE8 (and below) will parse trailing commas in array and object literals incorrectly. If you are targeting newer versions of JS, set the appropriate language_in option.
								type: 'textarea',
								                ^

ago 28, 2020 1:39:32 PM com.google.javascript.jscomp.LoggerErrorManager println
ADVERTENCIA: codemirrordialog.js:168: WARNING - Parse error. IE8 (and below) will parse trailing commas in array and object literals incorrectly. If you are targeting newer versions of JS, set the appropriate language_in option.
								type: 'html',
								            ^

ago 28, 2020 1:39:32 PM com.google.javascript.jscomp.LoggerErrorManager println
ADVERTENCIA: codemirrordialog.js:169: WARNING - Parse error. IE8 (and below) will parse trailing commas in array and object literals incorrectly. If you are targeting newer versions of JS, set the appropriate language_in option.
							},
							 ^

ago 28, 2020 1:39:32 PM com.google.javascript.jscomp.LoggerErrorManager println
ADVERTENCIA: codemirrordialog.js:171: WARNING - Parse error. IE8 (and below) will parse trailing commas in array and object literals incorrectly. If you are targeting newer versions of JS, set the appropriate language_in option.
						type: 'hbox',
						            ^

ago 28, 2020 1:39:32 PM com.google.javascript.jscomp.LoggerErrorManager println
ADVERTENCIA: codemirrordialog.js:172: WARNING - Parse error. IE8 (and below) will parse trailing commas in array and object literals incorrectly. If you are targeting newer versions of JS, set the appropriate language_in option.
					},
					 ^

ago 28, 2020 1:39:32 PM com.google.javascript.jscomp.LoggerErrorManager println
ADVERTENCIA: codemirrordialog.js:173: WARNING - Parse error. IE8 (and below) will parse trailing commas in array and object literals incorrectly. If you are targeting newer versions of JS, set the appropriate language_in option.
				],
				 ^

ago 28, 2020 1:39:32 PM com.google.javascript.jscomp.LoggerErrorManager println
ADVERTENCIA: codemirrordialog.js:174: WARNING - Parse error. IE8 (and below) will parse trailing commas in array and object literals incorrectly. If you are targeting newer versions of JS, set the appropriate language_in option.
			},
			 ^

ago 28, 2020 1:39:32 PM com.google.javascript.jscomp.LoggerErrorManager println
ADVERTENCIA: codemirrordialog.js:205: WARNING - Parse error. IE8 (and below) will parse trailing commas in array and object literals incorrectly. If you are targeting newer versions of JS, set the appropriate language_in option.
		},
		 ^

ago 28, 2020 1:39:32 PM com.google.javascript.jscomp.LoggerErrorManager printSummary
ADVERTENCIA: 0 error(s), 13 warning(s)
ago 28, 2020 1:39:32 PM com.google.javascript.jscomp.LoggerErrorManager println
ADVERTENCIA: plugin.js:22: WARNING - Parse error. IE8 (and below) will parse trailing commas in array and object literals incorrectly. If you are targeting newer versions of JS, set the appropriate language_in option.
						mode: 'text/html',
						                 ^

ago 28, 2020 1:39:32 PM com.google.javascript.jscomp.LoggerErrorManager println
ADVERTENCIA: plugin.js:107: WARNING - Parse error. IE8 (and below) will parse trailing commas in array and object literals incorrectly. If you are targeting newer versions of JS, set the appropriate language_in option.
							wysiwyg: 0,
							          ^

ago 28, 2020 1:39:32 PM com.google.javascript.jscomp.LoggerErrorManager println
ADVERTENCIA: plugin.js:109: WARNING - Parse error. IE8 (and below) will parse trailing commas in array and object literals incorrectly. If you are targeting newer versions of JS, set the appropriate language_in option.
						state: CKEDITOR.TRISTATE_OFF,
						                            ^

ago 28, 2020 1:39:32 PM com.google.javascript.jscomp.LoggerErrorManager println
ADVERTENCIA: plugin.js:123: WARNING - Parse error. IE8 (and below) will parse trailing commas in array and object literals incorrectly. If you are targeting newer versions of JS, set the appropriate language_in option.
					toolbar: 'mode,10',
					                  ^

ago 28, 2020 1:39:32 PM com.google.javascript.jscomp.LoggerErrorManager println
ADVERTENCIA: plugin.js:129: WARNING - Parse error. IE8 (and below) will parse trailing commas in array and object literals incorrectly. If you are targeting newer versions of JS, set the appropriate language_in option.
					toolbar: 'mode,11',
					                  ^

ago 28, 2020 1:39:32 PM com.google.javascript.jscomp.LoggerErrorManager println
ADVERTENCIA: plugin.js:142: WARNING - Parse error. IE8 (and below) will parse trailing commas in array and object literals incorrectly. If you are targeting newer versions of JS, set the appropriate language_in option.
		},
		 ^

ago 28, 2020 1:39:32 PM com.google.javascript.jscomp.LoggerErrorManager println
ADVERTENCIA: plugin.js:151: WARNING - Parse error. IE8 (and below) will parse trailing commas in array and object literals incorrectly. If you are targeting newer versions of JS, set the appropriate language_in option.
				wysiwyg: 1,
				          ^

ago 28, 2020 1:39:32 PM com.google.javascript.jscomp.LoggerErrorManager println
ADVERTENCIA: plugin.js:163: WARNING - Parse error. IE8 (and below) will parse trailing commas in array and object literals incorrectly. If you are targeting newer versions of JS, set the appropriate language_in option.
			canUndo: false,
			              ^

ago 28, 2020 1:39:32 PM com.google.javascript.jscomp.LoggerErrorManager println
ADVERTENCIA: plugin.js:164: WARNING - Parse error. IE8 (and below) will parse trailing commas in array and object literals incorrectly. If you are targeting newer versions of JS, set the appropriate language_in option.
		},
		 ^

ago 28, 2020 1:39:32 PM com.google.javascript.jscomp.LoggerErrorManager println
ADVERTENCIA: plugin.js:165: WARNING - Parse error. IE8 (and below) will parse trailing commas in array and object literals incorrectly. If you are targeting newer versions of JS, set the appropriate language_in option.
	},
	 ^

ago 28, 2020 1:39:32 PM com.google.javascript.jscomp.LoggerErrorManager printSummary
ADVERTENCIA: 0 error(s), 10 warning(s)
    Time taken.....: 27.158seconds
Merging language files
    Time taken.....: 3.146seconds
Generating plugins sprite image
Building ckeditor.js
Minifying ckeditor.js
ago 28, 2020 1:40:04 PM com.google.javascript.jscomp.LoggerErrorManager println
ADVERTENCIA: ckeditor.js:46205: WARNING - Parse error. IE8 (and below) will parse trailing commas in array and object literals incorrectly. If you are targeting newer versions of JS, set the appropriate language_in option.
						mode: 'text/html',
						                 ^

ago 28, 2020 1:40:04 PM com.google.javascript.jscomp.LoggerErrorManager println
ADVERTENCIA: ckeditor.js:46290: WARNING - Parse error. IE8 (and below) will parse trailing commas in array and object literals incorrectly. If you are targeting newer versions of JS, set the appropriate language_in option.
							wysiwyg: 0,
							          ^

ago 28, 2020 1:40:04 PM com.google.javascript.jscomp.LoggerErrorManager println
ADVERTENCIA: ckeditor.js:46292: WARNING - Parse error. IE8 (and below) will parse trailing commas in array and object literals incorrectly. If you are targeting newer versions of JS, set the appropriate language_in option.
						state: CKEDITOR.TRISTATE_OFF,
						                            ^

ago 28, 2020 1:40:04 PM com.google.javascript.jscomp.LoggerErrorManager println
ADVERTENCIA: ckeditor.js:46306: WARNING - Parse error. IE8 (and below) will parse trailing commas in array and object literals incorrectly. If you are targeting newer versions of JS, set the appropriate language_in option.
					toolbar: 'mode,10',
					                  ^

ago 28, 2020 1:40:04 PM com.google.javascript.jscomp.LoggerErrorManager println
ADVERTENCIA: ckeditor.js:46312: WARNING - Parse error. IE8 (and below) will parse trailing commas in array and object literals incorrectly. If you are targeting newer versions of JS, set the appropriate language_in option.
					toolbar: 'mode,11',
					                  ^

ago 28, 2020 1:40:04 PM com.google.javascript.jscomp.LoggerErrorManager println
ADVERTENCIA: ckeditor.js:46325: WARNING - Parse error. IE8 (and below) will parse trailing commas in array and object literals incorrectly. If you are targeting newer versions of JS, set the appropriate language_in option.
		},
		 ^

ago 28, 2020 1:40:04 PM com.google.javascript.jscomp.LoggerErrorManager println
ADVERTENCIA: ckeditor.js:46334: WARNING - Parse error. IE8 (and below) will parse trailing commas in array and object literals incorrectly. If you are targeting newer versions of JS, set the appropriate language_in option.
				wysiwyg: 1,
				          ^

ago 28, 2020 1:40:04 PM com.google.javascript.jscomp.LoggerErrorManager println
ADVERTENCIA: ckeditor.js:46346: WARNING - Parse error. IE8 (and below) will parse trailing commas in array and object literals incorrectly. If you are targeting newer versions of JS, set the appropriate language_in option.
			canUndo: false,
			              ^

ago 28, 2020 1:40:04 PM com.google.javascript.jscomp.LoggerErrorManager println
ADVERTENCIA: ckeditor.js:46347: WARNING - Parse error. IE8 (and below) will parse trailing commas in array and object literals incorrectly. If you are targeting newer versions of JS, set the appropriate language_in option.
		},
		 ^

ago 28, 2020 1:40:04 PM com.google.javascript.jscomp.LoggerErrorManager println
ADVERTENCIA: ckeditor.js:46348: WARNING - Parse error. IE8 (and below) will parse trailing commas in array and object literals incorrectly. If you are targeting newer versions of JS, set the appropriate language_in option.
	},

If we're fine about getting tons of warnings during the build process we don't need this... 😅

@julien
Copy link
Contributor

julien commented Aug 28, 2020

ADVERTENCIA: codemirrordialog.js:28: WARNING

I like how it displays "warning" in two different languages.

I'm mostly OK with those warnings because when I read them I understand they are being emitted because of the use of trailing commas, but I think it can confuse people - so we can either add a note somewhere in the README when talking about building ckeditor

@wincent
Copy link
Contributor

wincent commented Aug 28, 2020

If you are targeting newer versions of JS, set the appropriate language_in option.

I wonder if there is an easy way for us to pass a language_in setting along to the closure compiler 🤔 — I doubt it though, based on the README for their builder tool.

Although look at this:

$ java -jar ./dev/builder/ckbuilder/2.3.2/ckbuilder.jar --help

CKBuilder 2.3.2

USAGE: java -jar ckbuilder.jar

[TASK: build release]
SYNOPSIS:
  --build SRC DST (options...)
      SRC  source folder
      DST  destination folder
      [OPTIONS]
      --build-config <FILE>     path to the file
      --version <NUMBER>        version number
      --revision <NUMBER>       revision number
      --overwrite               overwrite target folder if exists
      -s,--skip-omitted-in-build-config
                                exclude from release all plugins/skins
                                that are not specified in build-config
      --leave-js-unminified     leave javascript files as is:
                                merge, but do not minify.
      --leave-css-unminified    leave CSS files as is:
                                merge, but do not minify.
      --no-ie-checks            turn off warnings about syntax errors on
                                Internet Explorer, like trailing commas
      --core                    create only the core file (ckeditor.js)
      --no-zip                  do not create zip file
      --no-tar                  do not create tar.gz file
      --commercial              builds a package with commercial license

DESCRIPTION:
  Creates CKEditor build in DST folder using source files from SRC folder.
  The build configuration file (build-config.js), which is required in order to
  create the build package, contains the list of plugins to include.

EXAMPLE:
  java -jar ckbuilder.jar --build ckeditor-dev release --version 4.0

[TASK: build skin]
SYNOPSIS:
  --build-skin SRC DST (options...)
      SRC  source folder
      DST  destination folder
      [OPTIONS]
      --overwrite               overwrite target folder if exists
      --leave-js-unminified     leave javascript files as is:
                                merge, but do not minify.
      --leave-css-unminified    leave CSS files as is:
                                merge, but do not minify.
      --no-ie-checks            turn off warnings about syntax errors on
                                Internet Explorer, like trailing commas

DESCRIPTION:
  Creates a release version of a skin.

EXAMPLE:
  java -jar ckbuilder.jar --build-skin skins/myskin target_dir

[TASK: create build configuration file]
SYNOPSIS:
  --generate-build-config SRC (options...)
      SRC  source folder
      [OPTIONS]
      --build-config <FILE>     path to the new file

DESCRIPTION:
  Creates build configuration file (default: build-config.js).

EXAMPLE:
  java -jar ckbuilder.jar --generate-build-config ckeditor-dev

[OTHER OPTIONS]

  -d,--debug-level <LEVEL> debug level (0, 1, 2).
  --help                   prints help information
  --build-help             prints help information about build configuration
  --full-help              prints help information about all advanced commands

Copyright (c) 2003-2020, CKSource - Frederico Knabben

Note this bit:

      --no-ie-checks            turn off warnings about syntax errors on
                                Internet Explorer, like trailing commas

If we can pass that through, then that might make this problem go away...

@julien
Copy link
Contributor

julien commented Aug 28, 2020

If we can pass that through, then that might make this problem go away...

Yes it does , if we add --no-ie-checks here then it goes away

@carloslancha
Copy link
Contributor Author

carloslancha commented Sep 1, 2020

So @wincent @julien I'm updating this pr to use the same lint config everywhere and add the no-ie-checks to the CKBuilder call. Sounds right?

@carloslancha
Copy link
Contributor Author

I'd also add to prettier config "quoteProps": "preserve", since we're getting this warning:

ADVERTENCIA: codemirrordialog.js:91: WARNING - Keywords and reserved words are not allowed as unquoted property names in older versions of JavaScript. If you are targeting newer versions of JavaScript, set the appropriate language_in option.
				class: 'cke_wysiwyg_frame cke_reset',
				^

@wincent
Copy link
Contributor

wincent commented Sep 1, 2020

Sounds right?

Yeah.

I'd also add to prettier config "quoteProps": "preserve", since we're getting this warning.

Up to you. If it is a single warning I'd probably just ignore it and avoid the clutter of a second config. But if you want to have the other config, feel free.

@carloslancha carloslancha changed the title chore: format plugins chore: add plugins files to format script Sep 1, 2020
@carloslancha
Copy link
Contributor Author

Done

Copy link
Contributor

@wincent wincent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fire-at-will-commander

@carloslancha
Copy link
Contributor Author

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

Successfully merging this pull request may close these issues.

3 participants