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

[Docs] update output options in readme #1958

Merged
merged 3 commits into from
May 17, 2017
Merged

[Docs] update output options in readme #1958

merged 3 commits into from
May 17, 2017

Conversation

Mottie
Copy link
Contributor

@Mottie Mottie commented May 16, 2017

Fix for PR #1957

  • Sorted the output options.
  • Updated default options for beautify, inline_script and max_line_len.
  • Added comments, ie8, preserve_line, shebang, source_map, unescape_regexps and wrap_iife.

README.md Outdated
comments, `"some"` to preserve some comments, a regular expression string
(e.g. `/^!/`) or a function.
- `ie8` (default `false`) -- pass `true` to support non-standard Internet
Explorer 8
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please remove ie8 here as suggested in #1957 (comment)

Users should be using options.ie8 under https://github.com/mishoo/UglifyJS2#minify-options

README.md Outdated
semicolon, leading to more readable output of uglified code (size before
gzip could be smaller; size after gzip insignificantly larger).
- `shebang` (default `true`) -- preserve shebang `#!` in preamble (bash scripts)
- `source_map` (default `null`) -- pass a `SourceMap` object to generate.
Copy link
Collaborator

Choose a reason for hiding this comment

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

options.output.source_map is an internal flag used by options.sourceMap, thus no need to be documented.

@alexlamsl
Copy link
Collaborator

@Mottie thanks for the work 👍

Looks fine to me except for the two comments above.

@kzc
Copy link
Contributor

kzc commented May 17, 2017

Please don't document unescape_regexps:

$ echo 'var a = /\uABCD\uEF/;' | bin/uglifyjs -b ascii_only
var a = /\uABCD\uEF/;

$ echo 'var a = /\uABCDE\uEF/;' | bin/uglifyjs -b unescape_regexps
var a = /ꯍE\uEF/;

I think the code for it should be dropped.

@alexlamsl
Copy link
Collaborator

#54 is part of the reason why unescape_regexps is introduced. But as mentioned in #1929 (comment), let's stay out of that game.

I'll put together a PR to remove unescape_regexps shortly.

@Mottie
Copy link
Contributor Author

Mottie commented May 17, 2017

Updated.

@alexlamsl
Copy link
Collaborator

@Mottie LGTM - thanks!

@kzc
Copy link
Contributor

kzc commented May 17, 2017

@alexlamsl Should we drop space_colon? It does not work outside of beautify=true, and even there it's of dubious value:

$ echo 'L:while(1){foo(x?y:{a:1});break L;}' | uglifyjs -b beautify=false,space_colon=false
L:while(1){foo(x?y:{a:1});break L}
$ echo 'L:while(1){foo(x?y:{a:1});break L;}' | uglifyjs -b beautify=false,space_colon=true
L:while(1){foo(x?y:{a:1});break L}
$ echo 'L:while(1){foo(x?y:{a:1});break L;}' | uglifyjs -b beautify=true,space_colon=true
L: while (1) {
    foo(x ? y : {
        a: 1
    });
    break L;
}
$ echo 'L:while(1){foo(x?y:{a:1});break L;}' | uglifyjs -b beautify=true,space_colon=false
L:while (1) {
    foo(x ? y :{
        a:1
    });
    break L;
}

Edit: in beautify=true mode, a space should always be emitted with the colon.

@alexlamsl
Copy link
Collaborator

@kzc I have no idea why space_colon was there, and I have explicitly set it to false myself to avoid any doubts in my use cases.

May I investigate further then put together a PR to tackle that instead of blocking this PR?

@kzc
Copy link
Contributor

kzc commented May 17, 2017

May I investigate further then put together a PR to tackle that instead of blocking this PR?

It's your call as the maintainer but I think this PR should remove space_colon from the documentation. We don't want to give anyone the idea that it'll be supported. :-)

@kzc
Copy link
Contributor

kzc commented May 17, 2017

Be aware that README commits are instantaneously shown on github and are independent of releases.

@alexlamsl
Copy link
Collaborator

@kzc alright, you win 🤣

@Mottie would you mind removing space_colon from this PR?

This was referenced May 17, 2017
@alexlamsl alexlamsl merged commit baef8bf into mishoo:master May 17, 2017
@alexlamsl
Copy link
Collaborator

@Mottie thanks for (literally) sorting this!

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.

None yet

3 participants