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

Using tabs when wrapping attributes and wrapping if needed #1294

Closed
ManuelDeLeon opened this issue Nov 21, 2017 · 7 comments
Closed

Using tabs when wrapping attributes and wrapping if needed #1294

ManuelDeLeon opened this issue Nov 21, 2017 · 7 comments

Comments

@ManuelDeLeon
Copy link

I was wondering if you're open to receiving two PRs to add two options for HTML.

  1. An option to use tabs when wrapping parameters + indent_with_tabs:true. Right now it adds spaces, creating a mixed mode file. Something like wrap_attributes_with_tabs:bool.

  2. An option to wrap parameters if they reach the wrap_line_length. Something similar to force-expand-multi-line but not forced. I don't have a good name for this option because I don't understand quite well what auto means for wrap_attributes.

We'd end up with a file formatted this way:

<div
	id="main"
	class="outer"
	data-one="long text to wrap"
	data-two="long text to wrap"
>
	<div id="sub" class="inner">
		content
	</div>
</div>

This is to match a common JS/TS style used when wrapping parameters:

public test(
	oneVeryLongParameter: string,
	twoLongParameters: string,
	threeLongParameters: string
) {
	console.log("me");
}

Thanks

@bitwiseman
Copy link
Member

@ManuelDeLeon - We're open to PRs, but please file these two different enhancement proposals as two differ issues (keep one here and open a new issue for the other).

Also, before implementing, take a minute to describe the behaviors more fully. Why would wrapping with tabs be better than spaces (if spaces are used for the rest of the file)?

@ManuelDeLeon
Copy link
Author

ManuelDeLeon commented Nov 27, 2017 via email

@bitwiseman
Copy link
Member

@ManuelDeLeon Spaces instead of tabs: Oh! If the rest fo the file already uses tabs then it would totally make sense for the wrapped attributes to use tabs as well - no additional option needed. I would say it is a bug that the wrap indent doesn't do this already.

Note, using tabs for indent would probably need to disable some of the other wrap settings like force-aligned.

@ManuelDeLeon
Copy link
Author

I thought it was intentional given this test:

        name: "Attribute Wrap alignment with spaces",
        description: "Ensure attributes are internally aligned with spaces when the indent_character is set to tab",
        matrix: [{
            options: [
                { name: "wrap_attributes", value: "'force-aligned'" },
                { name: "indent_with_tabs", value: "true" }
            ]
        }],
        tests: [{
            fragment: true,
            input: '<div><div a="1" b="2"><div>test</div></div></div>',
            output: '<div>\n\t<div a="1"\n\t     b="2">\n\t\t<div>test</div>\n\t</div>\n</div>'
        }]

Do you have a guide for running the tests?

@bitwiseman
Copy link
Member

@ManuelDeLeon
That test is probably just describing the current behavior and making sure it doesn't break further.

To run the tests see:
https://github.com/beautify-web/js-beautify/blob/master/CONTRIBUTING.md

@bitwiseman bitwiseman changed the title Using tabs when wrapping parameters and wrapping if needed Using tabs when wrapping attributes and wrapping if needed Dec 6, 2017
@bitwiseman
Copy link
Member

This is related to #1551.

@bitwiseman
Copy link
Member

@ManuelDeLeon
You request for wrapping when reaching a specific line length please open this as a separate bug. It might already be requested see #1404 and related.

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

No branches or pull requests

2 participants