Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Addresses #13264 where setting saveFoldStates preference to false caused folding problems #13269

Conversation

thehogfather
Copy link
Contributor

This PR properly initialises the _lineFolds cache to the empty object when the saveFoldStates preferences is set to false. Previously, this was not being done and it caused issues down stream, i.e., when rendering gutters as the active editor changed.

Also updated the test cases to accommodate the requirement.

cc @lkcampbell @ficristo

…lse caused the code folding extension to stop working.
@lkcampbell
Copy link
Contributor

@thehogfather, the changes look good to me. Initialize the uninitialized property with an empty object and get rid of the undefined/null error message down the road.

Is _codeMirror._lineFolds where you are storing all of the fold state information for each instance of CodeMirror opened in brackets?

The only problem I see is the Travis build is failing. @ficristo or @petetnt any ideas what might be causing the build fail?

@thehogfather
Copy link
Contributor Author

@lkcampbell

Is _codeMirror._lineFolds where you are storing all of the fold state information for each instance of CodeMirror opened in brackets?

Yes that is correct.

With respect to Travis failing, I'll pull upstream and push to trigger the build again. The error messages show it did not get very far into the build process.

…gfather/issue-13264-save-fold-states-error
@thehogfather
Copy link
Contributor Author

That did no seem to help. Let me know if there is anything I can do.

@petetnt
Copy link
Collaborator

petetnt commented Apr 6, 2017

Restarted the CI by hand, I'll look into it if it still fails

@swmitra
Copy link
Collaborator

swmitra commented Apr 6, 2017

I was just trying out with the OAuth token and couldn't exceed the limit in 60 attempts. Not sure why this one failed!

@petetnt
Copy link
Collaborator

petetnt commented Apr 6, 2017

We fixed the CI issue 👍

@ficristo ficristo added this to the Release 1.10 milestone Apr 6, 2017
@ficristo
Copy link
Collaborator

ficristo commented Apr 6, 2017

Fixes #13264 (adding this comment so GitHub creates a link to the issue)

@lkcampbell
Copy link
Contributor

lkcampbell commented Apr 7, 2017

Okay, I checked that the fix works and ran the unit tests: no errors.

Merging.

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

Successfully merging this pull request may close these issues.

None yet

5 participants