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

Add support for max_line_length #57

Closed
wants to merge 2 commits into from
Closed

Add support for max_line_length #57

wants to merge 2 commits into from

Conversation

joaoe
Copy link

@joaoe joaoe commented Sep 8, 2016

* Support for this feature requires the user to have installed the autowrap
  plugin https://github.com/randy3k/AutoWrap
@sindresorhus sindresorhus changed the title Introducing support for max_line_length. Add support for max_line_length Nov 4, 2016
@@ -103,4 +104,14 @@ def apply_config(self, view, config):
elif insert_final_newline == 'false':
settings.set('ensure_newline_at_eof_on_save', False)

if isinstance(max_line_length, int) and max_line_length:
Copy link
Owner

Choose a reason for hiding this comment

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

I think max_line_length is always a string, so you'll need to coerce it into an int.

Copy link
Author

@joaoe joaoe Nov 5, 2016

Choose a reason for hiding this comment

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

Wouldn't it be an int if typed as max_line_length = 123 instead of max_line_length = "123" ?

But I agree it can be made more flexible by parsing a string, if that's the case.

Copy link
Author

Choose a reason for hiding this comment

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

Ok, just tested it again. Indeed it's true. I think I tested it first by having a max length in my user preferences.

Thanks for the heads up.

Copy link
Owner

Choose a reason for hiding this comment

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

Ideally yes, but .editorconfig is based on .ini, which is untyped and unspec'd, so everything are just strings.

@sindresorhus
Copy link
Owner

I thought it was still not working at first, then I realized it only works on new text. Any way to get it to auto wrap existing text? (That's how the other rules work).

@sindresorhus
Copy link
Owner

And why does it require a plugin for this? Sublime already has Edit => Wrap.

@joaoe
Copy link
Author

joaoe commented Nov 5, 2016

I thought it was still not working at first, then I realized it only works on new text.

Because of if not view.settings().has(self.MARKER): which is not my code. That prevents applying preferences over and over again to views that gain focus. A better idea would perhaps be to store a timestamp, and then compare with the timestamp of the .editorconfig file or timestamp of the EditorConfig.py file (in case of a new version that changes behavior).

And why does it require a plugin for this? Sublime already has Edit => Wrap.

That, I didn't know... :) Will most likely require rewriting everything.

@sindresorhus
Copy link
Owner

Closing this for lack of activity. Happy to reopen whenever.

@joaoe
Copy link
Author

joaoe commented Jan 10, 2017

It's fine, I don't have to look at it now.

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.

None yet

2 participants