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

Allow the Utf-8 BOM with no-irregular-whitespace #4878

Closed
AlfonsoML opened this Issue Jan 7, 2016 · 12 comments

Comments

Projects
None yet
6 participants
@AlfonsoML
Copy link

commented Jan 7, 2016

As suggested in https://groups.google.com/forum/#!topic/eslint/UKq91-aGycw

A file that has the UTF-8 BOM triggers an error with the no-irregular-whitespace rule:
line 1, col 1, Error - Irregular whitespace not allowed (no-irregular-whitespace)

So this is a request to be able (at least as an option) to ignore the BOM at the start of the file (but not in any other place).

@eslintbot eslintbot added the triage label Jan 7, 2016

@eslintbot

This comment has been minimized.

Copy link

commented Jan 7, 2016

@AlfonsoML Thanks for the issue! If you're reporting a bug, please be sure to include:

  1. The version of ESLint you are using (run eslint -v)
  2. What you did (the source code and ESLint configuration)
  3. The actual ESLint output complete with numbers
  4. What you expected to happen instead

Requesting a new rule? Please see Proposing a New Rule for instructions.

@nzakas nzakas added bug core accepted and removed triage labels Jan 7, 2016

@nzakas

This comment has been minimized.

Copy link
Member

commented Jan 7, 2016

We really should strip the BOM before parsing, as most engines do that already.

@platinumazure

This comment has been minimized.

Copy link
Member

commented Jan 8, 2016

👍 from me- I really want to use this rule at work but we put the BOM on most of our files.

@AlfonsoML

This comment has been minimized.

Copy link
Author

commented Jan 8, 2016

Keep in mind that the --fix option should preserve the BOM when rewriting
the file, so it can't be fully dropped.
El 7/1/2016 11:36 p. m., "Nicholas C. Zakas" notifications@github.com
escribió:

We really should strip the BOM before parsing, as most engines do that
already.


Reply to this email directly or view it on GitHub
#4878 (comment).

@nzakas

This comment has been minimized.

Copy link
Member

commented Jan 8, 2016

Good point, we need to keep an eye out for that.

@mysticatea

This comment has been minimized.

Copy link
Member

commented Jan 11, 2016

I will try to fix this.

My plan:

  • Remove BOM from the text at the head of eslint.verify() if a text was given.
  • Add hasBOM boolean property to SourceCode object.
    I will add the third argument (options): {hasBOM: boolean}.
  • If SourceCode#hasBOM is true, SourceCodeFixer inserts BOM to the head of output.
  • Add insertBOM and removeBOM methods to RuleFixer (for a rule like unicode-bom (never/always, fixable)).

Thought?

@ilyavolodin

This comment has been minimized.

Copy link
Member

commented Jan 11, 2016

We should probably do something similar to hashbangs, since we just remove them currently, but do not add them back.

@nzakas

This comment has been minimized.

Copy link
Member

commented Jan 11, 2016

@ilyavolodin can you open a separate issue for that?

@mysticatea I don't think insertBOM/removeBOM is necessary. Otherwise, sounds good.

@ilyavolodin

This comment has been minimized.

Copy link
Member

commented Jan 11, 2016

Sure. Opened #4916 for hashbangs.

@mysticatea

This comment has been minimized.

Copy link
Member

commented Jan 13, 2016

@nzakas In this approach, rules cannot remove BOM with --fix, and I have a rule that I want to remove BOM with --fix.

Could we add insertBOM/removeBOM?

mysticatea added a commit to mysticatea/eslint that referenced this issue Jan 13, 2016

Fix: Supports Unicode BOM (fixes eslint#4878)
- `eslint.verify()` API came to strip BOM before parsing.
- `hasBOM` property was added into `SourceCode` object.
- `SourceCodeFixer.applyFixes()` came to insert BOM to the head of output if `sourceCode.hasBOM` is `true`.
@nzakas

This comment has been minimized.

Copy link
Member

commented Jan 13, 2016

Can you add/remove using insertBeforeRange or removeRange?

I just really don't like the idea of having such specific methods.

@mysticatea

This comment has been minimized.

Copy link
Member

commented Jan 14, 2016

Though I'm guessing fixer.removeRange([-1, 0]) can be used for BOM even if BOM was stripped before parsing, it's your intention?

mysticatea added a commit to mysticatea/eslint that referenced this issue Jan 14, 2016

Fix: Supports Unicode BOM (fixes eslint#4878)
- `eslint.verify()` API came to strip BOM before parsing.
- `SourceCode` constructor came to strip BOM of `text` argument.
- `hasBOM` property was added into `SourceCode` object.
- `SourceCodeFixer.applyFixes()` came to insert BOM to the head of output if `sourceCode.hasBOM` is `true`.

mysticatea added a commit to mysticatea/eslint that referenced this issue Jan 15, 2016

Fix: Supports Unicode BOM (fixes eslint#4878)
- `eslint.verify()` API came to strip BOM before parsing.
- `SourceCode` constructor came to strip BOM of `text` argument.
- `hasBOM` property was added into `SourceCode` object.
- `SourceCodeFixer.applyFixes()` came to insert BOM to the head of output if `sourceCode.hasBOM` is `true`.

mysticatea added a commit to mysticatea/eslint that referenced this issue Jan 16, 2016

Breaking: Supports Unicode BOM (fixes eslint#4878)
- `eslint.verify()` API came to strip BOM before parsing.
- `SourceCode` constructor came to strip BOM of `text` argument.
- `hasBOM` property was added into `SourceCode` object.
- `SourceCodeFixer.applyFixes()` came to insert BOM to the head of output if `sourceCode.hasBOM` is `true`.

@nzakas nzakas closed this in #4935 Jan 18, 2016

nzakas added a commit that referenced this issue Jan 18, 2016

Merge pull request #4935 from mysticatea/core/supports-bom
Fix: Supports Unicode BOM (fixes #4878)

@eslint eslint bot locked and limited conversation to collaborators Feb 6, 2018

@eslint eslint bot added the archived due to age label Feb 6, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.