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

Appended extra lines in the bottom of Minimap #292

Closed
fundon opened this issue Mar 6, 2015 · 10 comments
Closed

Appended extra lines in the bottom of Minimap #292

fundon opened this issue Mar 6, 2015 · 10 comments

Comments

@fundon
Copy link
Member

fundon commented Mar 6, 2015

issue

I try to fix this.
Comment these lines: https://github.com/atom-minimap/minimap/blob/master/lib/mixins/canvas-drawer.coffee#L430-L435
And it isn't reproduce.

Note: Just only READMe.md file can reproduce it.

@abe33
Copy link
Contributor

abe33 commented Mar 11, 2015

If you're able to write a test that trigger that behavior so that we can make sure that the fix you're proposing works as expected without breaking anything it'ld be great.

@fundon
Copy link
Member Author

fundon commented Mar 11, 2015

I will add a test in few days.

@fundon
Copy link
Member Author

fundon commented Mar 16, 2015

@abe33
Atom will enable Soft Wrap automagically when openning Markdown Preview, and it will be reproduced.
BTW, as long as enable Soft Wrap that will be reproduced.

@abe33
Copy link
Contributor

abe33 commented Mar 16, 2015

As far as I can tell from your gif, the soft wrap is already enabled. But my guess is that you haven't a preferred line length defined.

@fundon
Copy link
Member Author

fundon commented Mar 16, 2015

It defatuls to 80. Not need.

@fundon
Copy link
Member Author

fundon commented Mar 16, 2015

Maybe we can use TextEditor#isSoftWrapped to check the status.

@abe33
Copy link
Contributor

abe33 commented Mar 16, 2015

I'm not sure to understand what you're talking about. In the gif you posted, the soft wrap is already enabled before splitting the pane, so testing isSoftWrapped will return true at both times.
Also we relies on the text editor screen lines so there's should be no need for us to deal with soft wrapped lines in a special way.

@fundon
Copy link
Member Author

fundon commented Mar 16, 2015

It's working for me when commet these linnes https://github.com/atom-minimap/minimap/blob/master/lib/mixins/canvas-drawer.coffee#L440-L445.

But I'm not clear.
I'm closing this issue. If someone has same issue, reopen it.
:)

@fundon fundon closed this as completed Mar 16, 2015
@abe33
Copy link
Contributor

abe33 commented Mar 16, 2015

I'm not saying there is no issue, your gif is telling enough, but I'm cautious with untested changes like 'comment these lines and it works'.
We need tests that reproduce this issue so that we can make sure the fix really solves the issue without introducing new bugs elsewhere.

@abe33 abe33 reopened this Jun 4, 2015
@abe33
Copy link
Contributor

abe33 commented Jun 4, 2015

So I somehow figured what the cause was, it also seems I found a fix for that so I reopen this issue to keep track of the changes.

@abe33 abe33 closed this as completed in e048e16 Jun 4, 2015
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

No branches or pull requests

2 participants