Commit Graph

5 Commits (952bc0cc039c034c5fe651864db528dcd9800e9e)

Author SHA1 Message Date
Ika 952bc0cc03
fix: get rid of CRLF (#5494) 2018-12-08 18:28:29 +08:00
Joseph Frazier d4217f5508 Unescape unnecessarily escaped characters in strings (#1575)
* Add test with unnecessarily escaped non-quote character

In 0b6d19db18,
I noticed that `makeString` doesn't unescape unnecessarily escaped
non-quote characters. This change simply adds a test for that.

* Fix test with unnecessarily escaped non-quote character

Unfortunately, this breaks a couple of other tests...

* Revert "Fix test with unnecessarily escaped non-quote character"

See https://github.com/prettier/prettier/pull/1575#issuecomment-300490172

This reverts commit d05652518fe7d4e2fb82ce48ffc922b153de5593.

* Unescape unnecessarily escaped characters in strings

* Add test for unnecessarily escaped character at not-beginning of string

* Fix test for unnecessarily escaped character at not-beginning of string

* Add test for multiple unnecessary escapes in strings

* Pass test for multiple unnecessary escapes in strings

* Add test for octal escapes in strings

See https://github.com/prettier/prettier/pull/1575#discussion_r115804065

* Pass test for octal escapes in strings

See https://github.com/prettier/prettier/pull/1575#discussion_r115804065

* Add test for unnecessarily escaped character preced by escaped backslash

See https://github.com/prettier/prettier/pull/1575#discussion_r115804065

* Pass test for unnecessarily escaped character preced by escaped backslash

This just allows an even number of backslashes to precede the
unnecessary one.

See https://github.com/prettier/prettier/pull/1575#discussion_r115804065

* Add test for unescaped character after escaped backslash in strings

* Add test for unescaped character preceded by two escaped backslashes in string

See https://github.com/prettier/prettier/pull/1575#discussion_r115808571

* Pass test for unescaped character preceded by two escaped backslashes in string

This breaks another test though...

See https://github.com/prettier/prettier/pull/1575#discussion_r115808571

* Update snapshot

It turns out the test wasn't broken, I had just flubbed the escaping in
the snapshot. The easiest way to see that this actually works is

```bash
$ cat | prettier --stdin
"hol\\a (the a is not escaped)" // press Control-D after the newline
"hol\\a (the a is not escaped)"; // press Control-D after the newline
```

* Prevent test strings from being parsed as directives

See https://github.com/prettier/prettier/pull/1575#discussion_r115820336

(cherry picked from commit 126e56ab2c79801cbf7fee22189212d8c93581df)

* Add test for consecutive unnecessarily escaped characters in strings

See https://github.com/prettier/prettier/pull/1575#discussion_r115822286

* Pass test for consecutive unnecessarily escaped characters in strings

See https://github.com/prettier/prettier/pull/1575#discussion_r115822286

This looping is hacky. We might be able to emulate lookbehind instead.

* Optimize (maybe?) string unescaping loop

Not sure how expensive string comparison is here...

See 2323c8c025 (commitcomment-22092267)

* Safeguard against string unescaping loop hanging

See https://github.com/prettier/prettier/pull/1575#discussion_r115827531

* Add more comprehensive tests for unnecessary string escapes

See https://github.com/prettier/prettier/pull/1575#discussion_r115798155

* Remove superfluous variables from makeString()

See https://github.com/prettier/prettier/pull/1575#discussion_r115834468

* Unescape unnecessary strings escapes without looping

* Unescape unnecessary string escapes while handling quotes

Kudos to @lydell for figuring this out! See
https://github.com/prettier/prettier/pull/1575/files#r115860741

* Test that unnecessary escapes remain in directive literals

See:
* https://github.com/prettier/prettier/pull/1575#discussion_r115820336
* https://github.com/prettier/prettier/pull/1575#issuecomment-300633277
2017-05-10 16:02:49 -07:00
Joseph Frazier 8cc38ad524 Preserve code unit sequence of directive literals (#1571)
* Print directive literals verbatim

This addresses https://github.com/prettier/prettier/issues/1555,
but doesn't seem to pass the AST_COMPARE=1 tests:

    AST_COMPARE=1 npm test -- tests/quotes -t strings

However, running `prettier --debug-check` on the relevant file *does*
work:

    prettier tests/quotes/strings.js --debug-check

* Change directive literal quotes if it doesn't contain quotes

This addresses https://github.com/prettier/prettier/pull/1560#discussion_r115396257

From https://github.com/prettier/prettier/issues/1555#issue-227206837:

> It's okay to change the type of quotation marks used, but only if
doing so does not require changing any characters within the directive.

* Don't change directive literal quotes if it contains a backslash

This passes the `--debug-check` tests again:

    prettier tests/quotes/strings.js --debug-check

* Try to add regression test for escaped directive literals

This seems not to work, despite the following command having the correct
output:

    echo "'\''" | prettier

You can use the following to get an idea of how flow/typescript parse
this:

    node -p "JSON.stringify(require('./src/parser').parse('\\'\\\\\'\\'', {parser: 'flow'}), null, 2)"
    node -p "JSON.stringify(require('./src/parser').parse('\\'\\\\\'\\'', {parser: 'typescript'}), null, 2)"

* WIP Disable Flow/Typescript for ./tests/directives

We don't yet handle escaped directives for them, but Babylon works.

(similar to 90bf93713c (diff-0de18284f37da79ab8af4e4690919abaR1))

* Revert "WIP Disable Flow/Typescript for ./tests/directives"

This reverts commit 2aba6231271f6985a395c31e3df9323e8f3da115.

* Prevent test strings from being parsed as directives

See https://github.com/prettier/prettier/pull/1560#issue-227225960

* Add more escaped directive tests

* Infer DirectiveLiterals from Flow parser

* Don't test TypeScript on directives

See https://github.com/prettier/prettier/pull/1560#issuecomment-300296221

* fixup! Infer DirectiveLiterals from Flow parser

* Don't fake objects that look like a DirectiveLiteral

Instead, add a flag to nodeStr() that deals with the Flow node
accordingly. See https://github.com/prettier/prettier/pull/1560#discussion_r115605758

* Print preferred quotes around escaped DirectiveLiteral when it doesn't contain quotes

See https://github.com/prettier/prettier/pull/1560#discussion_r115606122

* Simplify `canChangeDirectiveQuotes` logic

* Add directive test with unnecessarily escaped non-quote character

* Fix boolean logic error

I thought that this would result in the following if-block executing, as
needed to pass the test case in the previous commit. However, it appears
that it's not actually needed to pass the test case, since `makeString`
doesn't unescape unnecessarily escaped non-quote characters.
Nevertheless, I think we should leave that if-block (`if (canChangeDirectiveQuotes)`)
there, in case `makeString` is updated.

See https://github.com/prettier/prettier/pull/1571#discussion_r115658398

* Make isFlowDirectiveLiteral a separate argument to nodeStr()

See https://github.com/prettier/prettier/pull/1571#discussion_r115810988

* Simplify isFlowDirectiveLiteral logic by passing n.expression to nodeStr()

See https://github.com/prettier/prettier/pull/1571#discussion_r115811216
2017-05-10 12:15:27 -07:00
Christopher Chedeau 13f247aee3 Revert "Print directive literals verbatim (#1560)" (#1570)
This reverts commit 32d9d68154.
2017-05-09 14:19:03 -07:00
Joseph Frazier 32d9d68154 Print directive literals verbatim (#1560)
* Print directive literals verbatim

This addresses https://github.com/prettier/prettier/issues/1555,
but doesn't seem to pass the AST_COMPARE=1 tests:

    AST_COMPARE=1 npm test -- tests/quotes -t strings

However, running `prettier --debug-check` on the relevant file *does*
work:

    prettier tests/quotes/strings.js --debug-check

* Change directive literal quotes if it doesn't contain quotes

This addresses https://github.com/prettier/prettier/pull/1560#discussion_r115396257

From https://github.com/prettier/prettier/issues/1555#issue-227206837:

> It's okay to change the type of quotation marks used, but only if
doing so does not require changing any characters within the directive.

* Don't change directive literal quotes if it contains a backslash

This passes the `--debug-check` tests again:

    prettier tests/quotes/strings.js --debug-check

* Try to add regression test for escaped directive literals

This seems not to work, despite the following command having the correct
output:

    echo "'\''" | prettier

You can use the following to get an idea of how flow/typescript parse
this:

    node -p "JSON.stringify(require('./src/parser').parse('\\'\\\\\'\\'', {parser: 'flow'}), null, 2)"
    node -p "JSON.stringify(require('./src/parser').parse('\\'\\\\\'\\'', {parser: 'typescript'}), null, 2)"

* WIP Disable Flow/Typescript for ./tests/directives

We don't yet handle escaped directives for them, but Babylon works.

(similar to 90bf93713c (diff-0de18284f37da79ab8af4e4690919abaR1))

* Revert "WIP Disable Flow/Typescript for ./tests/directives"

This reverts commit 2aba6231271f6985a395c31e3df9323e8f3da115.

* Prevent test strings from being parsed as directives

See https://github.com/prettier/prettier/pull/1560#issue-227225960

* Add more escaped directive tests

* Infer DirectiveLiterals from Flow parser

* Don't test TypeScript on directives

See https://github.com/prettier/prettier/pull/1560#issuecomment-300296221

* fixup! Infer DirectiveLiterals from Flow parser
2017-05-09 17:17:29 -04:00