Skip to content
This repository has been archived by the owner on Mar 3, 2023. It is now read-only.

Fix whacky soft-wrap issues #1492

Merged
merged 7 commits into from Feb 14, 2014
Merged

Fix whacky soft-wrap issues #1492

merged 7 commits into from Feb 14, 2014

Conversation

nathansobo
Copy link
Contributor

This PR adds:

  1. An editor spec that randomly mutates a buffer and toggles soft wrap on and off, then compares the screen lines to a simple reference implementation to ensure everything stays in a correct state.
  2. A new and radically simpler implementation of RowMap that eliminates all observed failures in the randomized test.

I feel pretty good about this now. The RowMap is way simpler now and I can't manage to get any of the failures I was previously driving out with the random spec. It's up to you guys whether you want to merge it while I'm gone. I think it might be worth it. I packaged it as a single commit so you can revert it if unexpected behaviors start cropping up.

It's worth noting that I added a dev dependency for an npm I'm using in the random spec. I'm not actually sure that's a thing. @kevinsawicki perhaps you can make an adjustment if necessary?

This commit adds two important things:

1. An editor spec that randomly mutates a buffer and toggles soft wrap
on and off, then compares the screen lines to a simple reference
implementation to ensure everything stays in a correct state.

2. A new and radically simpler implementation of RowMap that eliminates
failures in the randomized test.
@nathansobo
Copy link
Contributor Author

Fixes #1454

@kevinsawicki
Copy link
Contributor

Dev deps are fine in atom/atom

@probablycorey
Copy link

I can't say I understand everything going on based on reading the diff, but it looks easier to grok than before. I say we merge it and spend some extra time in softwrap mode.


map.spliceRegions(3, 7, [{bufferRows: 5, screenRows: 5}])

console.log map.inspect()
Copy link
Contributor

Choose a reason for hiding this comment

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

🔥 ?

@nathansobo
Copy link
Contributor Author

Had some downtime to address the comments in the code review and merge in master. I'll leave it to you guys to merge if and when you want to.

Nathan Sobo added 2 commits February 2, 2014 19:52
This was referenced Sep 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants