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

Create EndOfLine option #295

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Create EndOfLine option #295

wants to merge 3 commits into from

Conversation

virzak
Copy link
Contributor

@virzak virzak commented Aug 16, 2020

Description:

Add styler option end-of-line ( lf | crlf ) to force specific line endings

xstyler -p -r -d WPF -l Minimal --end-of-line lf

Partially fixes #285

Checklist:

  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have tested my changes by running the extension in VS2017
  • I have tested my changes by running the extension in VS2019
  • If changes to the documentation are needed, I have noted this in the description above

@Jay-o-Way
Copy link

What's keeping this?

Copy link
Contributor

@grochocki grochocki left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution! I am suggesting a slightly different approach that follows a pattern more similar to how we handle IndentSize that I would be interested in your feedback on.

src/XamlStyler/Options/IStylerOptions.cs Outdated Show resolved Hide resolved
src/XamlStyler.Console/Options.cs Show resolved Hide resolved
[Category("Misc")]
[RefreshProperties(RefreshProperties.All)]
[Description("Defines end of line character. Specify 'lf' or 'crlf'; otherwise, default character of the host will be used.")]
[JsonProperty("EndOfLine", DefaultValueHandling = DefaultValueHandling.IgnoreAndPopulate)]
Copy link
Contributor

Choose a reason for hiding this comment

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

For consistency with Visual Studio, can we call this "LineEndings"?

src/XamlStyler/Options/StylerOptions.cs Show resolved Hide resolved
src/XamlStyler/Options/StylerOptions.cs Outdated Show resolved Hide resolved
[Description("Defines end of line character. Specify 'lf' or 'crlf'; otherwise, default character of the host will be used.")]
[JsonProperty("EndOfLine", DefaultValueHandling = DefaultValueHandling.IgnoreAndPopulate)]
[DefaultValue("")]
public string EndOfLine
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of exposing this setting in the UI at all, I am wondering if this setting should follow the pattern of UseVisualStudioIndentSize/IndentSize (i.e., do not expose in settings UI, but still allow IDE to be overwritten in config file)?

Though, in this case, we wouldn't need UseVisualStudioLineEndings because we don't need to look it up from the XAML editor properties. Instead, if LineEndings is unset in a configuration file, NewLine is set to Environment.NewLine (current behavior), but then it could be overwritten. Instead of accepting a string, follow pattern of options like AttributeIndentationStyle and create an enum with: CRLF, LF, CR.

@virzak
Copy link
Contributor Author

virzak commented Jan 26, 2024

@grochocki, all issues are resolved apart from end-of-line vs line-ending. I'm cool with whatever you decide. Do you have a suggestion on automated testing for this PR?

@virzak
Copy link
Contributor Author

virzak commented Apr 12, 2024

@grochocki would it be possible to merge this soon?

@Sergio0694
Copy link

I'm also very interested in this. Is any help needed? @grochocki are there any blockers? 🙂

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.

Provide Line Ending option
4 participants