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

Fixes #162 #222

Merged
merged 1 commit into from
Oct 6, 2016
Merged

Fixes #162 #222

merged 1 commit into from
Oct 6, 2016

Conversation

jonathanrdelgado
Copy link
Contributor

@jonathanrdelgado jonathanrdelgado commented Oct 5, 2016

The regex wasn't matching the line. I think Atom use to includes newlines on to the beginning of text buffers, this is no longer the case. This fixes the Wrap Lines command.

Ref #162
cc @MoritzKn

Edit: Actually, looks like due to Atom's reporting system, #162 is actually an accumulation of multiple errors, all with the same error message Uncaught TypeError: Cannot read property '1' of null - this PR will resolve the issues regarding the wrap lines function, but not all reported in that thread. The issue should remain open.

The regex wasn't matching the line. I think Atom use to includes newlines on to the beginning of text buffers, this is no longer the case. This fixes the `Wrap Lines` command.
@MoritzKn
Copy link
Collaborator

MoritzKn commented Oct 6, 2016

Thanks for your PR! You're right #162 is a real mess. I tried to sort it out and added a comment which should give an overview.
Could you please explain how to reproduce the error you are fixing with this?

@jonathanrdelgado
Copy link
Contributor Author

@MoritzKn Yeah, sorry for not including the repro.

  • Add the following code to a JavaScript text editor. (This works in other languages as well)
/**
 * This is a horribly long comment that should wrap because I will have to scroll horizontally to see the entire thing because it is so long
 */
  • Have your Preferred Line Length in Atom settings be around 80, which is the default. Esentially, enough for the sample text to be longer than the line length.
  • Select anywhere in the sentence
  • Run the Docblockr: Wrap Lines command
  • Will throw Cannot read property '1' of null which can be traced back to the change made

I went through the stack trace, this line causes the error. I noticed that the text variable in that scope didn't have a leading line break, which caused the match on line 583 to fail.

As per the cause of this, I think Atom use to include a newline when getting buffer regions, this is no longer the case.

@jonathanrdelgado
Copy link
Contributor Author

jonathanrdelgado commented Oct 6, 2016

Also, was just searching for any other lingering issues on this matter, found #28 which actually implemented this same fix, but wasn't merged because it included another odd function - not sure what value it added over Wrap Lines at face value.

Fixes #16

@MoritzKn MoritzKn changed the base branch from master to develop October 6, 2016 16:59
@MoritzKn
Copy link
Collaborator

MoritzKn commented Oct 6, 2016

Ok, thank you! Good research, I appreciate that. 😄

@MoritzKn MoritzKn merged commit 38e010c into nikhilkalige:develop Oct 6, 2016
@MoritzKn
Copy link
Collaborator

MoritzKn commented Oct 6, 2016

Released with version v0.8.7.

@jonathanrdelgado
Copy link
Contributor Author

Thanks for the timely merge @MoritzKn!

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

Successfully merging this pull request may close these issues.

None yet

2 participants