Skip to content

Line break issues in some cases#375

Closed
ml1nk wants to merge 4 commits intojcubic:develfrom
ml1nk:devel
Closed

Line break issues in some cases#375
ml1nk wants to merge 4 commits intojcubic:develfrom
ml1nk:devel

Conversation

@ml1nk
Copy link
Copy Markdown
Contributor

@ml1nk ml1nk commented Mar 4, 2018

In situations where a line break is at the beginning inside a formatter like [[;;]\n] the line before the break gets ignored. For example A[[;;]\n]B becomes B without any sign of A.

The pull requests adds some tests to detect the problem an tries to correct it by adding/removing a char if necessary.

The problem is that $.terminal.iterate_formatting won't trigger an callback after the last bracket of a line when there is no character following. There are cases where it is correct (just remove 'array[i + 1][0] !== "["' to see the relevant cases fail with an empty line extra, but in our case it leads to ignoring a line.

By adding a char (for example " ") in some situations* we get the necessary callback. By removing the char and marking it as the last_iteraction it adds the line correctly.

There is probably a better way to fix it, but at least the origin of the bug is visible. If the fix isn`t working in all cases or a better way found, i will reduce the pull request to the tests only.

*

line[line.length - 1] === "]" && // ends without content
i < array.length - 1 && // there is a next entry (the line was separated with \n)
array[i + 1][0] !== "["  // \n was inside the formatting

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.07%) to 79.387% when pulling 3a80675 on ml1nk:devel into 9e757de on jcubic:devel.

@jcubic
Copy link
Copy Markdown
Owner

jcubic commented Mar 4, 2018

I think it's better to fix iterate_formatting then patch usage if it, I kind of fixed the issue with this snippet:

                } else if (i === string.length - 1) {
                    callback({
                        count: count,
                        index: i,
                        formatting: formatting,
                        length: 0,
                        text: in_text,
                        space: space
                    });
                }

it fixed the issue but break tests because it create empty string for normal cases, will try to fix the issue or just use filter(Boolean) if I'll don't found better solution.

jcubic added a commit that referenced this pull request Mar 4, 2018
@jcubic
Copy link
Copy Markdown
Owner

jcubic commented Mar 4, 2018

ok, I've fixed the empty string issue, I've just removed last_bracket check from split_equal the code is much nicer now.

@jcubic jcubic closed this Mar 4, 2018
@ml1nk ml1nk deleted the devel branch March 4, 2018 10:01
@ml1nk
Copy link
Copy Markdown
Contributor Author

ml1nk commented Mar 4, 2018

Thank you for the better bugfix, hopefully the PR saved you some time debugging.

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

Successfully merging this pull request may close these issues.

3 participants