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

SFC parseComponent pads complete content with spaces #5059

Conversation

sandersn
Copy link
Contributor

@sandersn sandersn commented Mar 2, 2017

SFC parseComponent pads all content with spaces when { pad: true } is provided. That is, all content is converted to spaces. Previously, each line was truncated to //. The new padding method works better with character-oriented tools that calculate positions by distance from the beginning of the file instead of by line
number.

Fixes #5058.

when `{ pad: true }` is provided. That is, all content is converted to
spaces. Previously, each line was truncated to "//". The new padding
method works better with character-oriented tools that calculate
positions by distance from the beginning of the file instead of by line
number.
Copy link
Member

@kazupon kazupon left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@kazupon kazupon requested a review from yyx990803 March 2, 2017 02:16
@yyx990803
Copy link
Member

yyx990803 commented Mar 3, 2017

This is an interesting change - one of the implications (and the reason why I used comments before) is when used with a linter that checks whitespaces or blank lines, this would raise a lot of warnings... (e.g. unnecessary whitespace / newlines)

Maybe we can change the option to use different values, e.g. { pad: 'space' } to use space mode, otherwise retain old behavior?

@sandersn
Copy link
Contributor Author

sandersn commented Mar 3, 2017

Good idea. I didn't think about the linter case. I'll change the type of pad to true | false | "space" | "line", where both "line" and true give the old padding behaviour.

Also still supports true for backward compatibility. True is the same as
"line".
sandersn added a commit to sandersn/vue-loader that referenced this pull request Mar 3, 2017
The pad option now accepts 'line' or 'space' as padding options.
Depends on vuejs/vue#5059
@yyx990803 yyx990803 merged commit 2dc177f into vuejs:dev Mar 5, 2017
yyx990803 pushed a commit to vuejs/vue-loader that referenced this pull request Mar 5, 2017
The pad option now accepts 'line' or 'space' as padding options.
Depends on vuejs/vue#5059
@sandersn sandersn deleted the sfc-parseComponent-completely-pads-content-with-spaces branch March 6, 2017 16:34
xalexec pushed a commit to Transnal/vue-loader that referenced this pull request Mar 26, 2017
The pad option now accepts 'line' or 'space' as padding options.
Depends on vuejs/vue#5059
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.

4 participants