Codefold preprocessor improvements #1054
Conversation
|
The state of the art coldfolding produces wrong results: |
|
Thanks. I will take a look at it. |
There was a problem hiding this comment.
Mainly a reasonable plan, but the tests are currently failing on zero-length lines (see comments). Could you also address the linting errors - see https://travis-ci.org/ipython-contrib/jupyter_contrib_nbextensions/jobs/266962119#L762-L773
| if indent <= fold_indent: | ||
| lstrip = l.lstrip(r' \t') # strip tabs and spaces | ||
| indent = len(l) - len(lstrip) | ||
| isSkipLine = lstrip[0] == "#" or lstrip in ["\n"] # is it a comment or an empty line |
There was a problem hiding this comment.
the lstrip[0] in this line is causing tests to fail. It should be something more like
# skip comments and empty lines, except at the end of the fold
# in which case they'll be replaced when the fold finishes
is_skip_line = lstrip.startswith('#') or lstrip == '\n'There was a problem hiding this comment.
sure :-), will take a look at the mistakes =)
There was a problem hiding this comment.
by the way, how do you reproduce the linter errors locally?
I am using atom, but flake8 is giving me not the same errors? Do you have a solution for this or you just look at travis?
| @@ -32,26 +32,52 @@ def fold_cell(self, cell, folded): | |||
| """ | |||
| Remove folded lines and add a '<->' at the parent line | |||
| """ | |||
| # self.log.debug("CodeFoldingPreprocessor:: folding at: %s" % folded) | |||
There was a problem hiding this comment.
I think debug logs can (should) stay in, since the default log level is info...
| lines = cell.splitlines(True) | ||
|
|
||
| if folded[0] == 0 and (lines[0][0] == '#' or lines[0][0] == '%'): | ||
| # fold whole cell when first line is a comment or magic | ||
| # self.log.debug("fold whole cell") |
There was a problem hiding this comment.
ditto above, we can keep this at debug level
|
So the output on Travis comes from flake8, run by tox. I just run flake8 locally in a terminal, and that's enough, but I guess it's possible that the atom version isn't looking for config in the same places as my standalone setup? Not sure at the moment sorry... |
|
Ah now I got the errors with atom, was on the wrong branch shit :-) Ehm -> Can line ending be '\r\n' like on Windows? in python notebooks? |
|
Hm, it seems its alway '\n' which is good |
I think notebook cells are stored as an array of lines, which makes them ending-agnostic, although the file itself can presumably have carriage returns... always best to assume the most difficult 😉 |
|
I checked on a notebook from windows and it seems its always '\n'. Having a json file which is not lineendings-agnostic would be a design mistake =), so notebook went the right path here I think :) |
|
Could we merge these changes =)? Thanks :-) |
|
Staring at https://travis-ci.org/ipython-contrib/jupyter_contrib_nbextensions/jobs/267700939#L762, there are still a bunch of linting errors? |
|
*starting |
|
ah ok :), they are only warning, thx that you are so persistent :)! makes the code better
Von meinem iPhone gesendet
… Am 31.08.2017 um 00:38 schrieb Josh Barnes ***@***.***>:
*starting
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
Ok, seems good to me, thanks @gabyx! |
This improvements make the preprocessor much more stable:
There were several bugs, such that outer levels did not get collapsed when exporting.
Now the coldfolding during export is closer to the experience in the notebook (if not totally the same)
Input:
Output: