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

bugfix for compact ellipsis on windows system #33

Closed
wants to merge 1 commit into from
Closed

bugfix for compact ellipsis on windows system #33

wants to merge 1 commit into from

Conversation

FelixErnst
Copy link
Contributor

On windows the compact ellipsis is now shown correctly due to encoding change in S4Vectors::safeExplode. Solution works both on Windows and Linux. macOS was not tested.

solution works both on Windows and Linux. macOS was not tested.
@FelixErnst
Copy link
Contributor Author

The windows specific error of the Modstrings package on devel is caused by the compact_ellipsis incompatibility on windows system triggered by the show method.

http://bioconductor.org/checkResults/3.11/bioc-LATEST/Modstrings/tokay2-buildsrc.html

Modstrings relies currently on the toSeqSnippet function from Biostrings. In this function the compact_ellipsis is added, but the encoding does not stick, so that the character get two additional letters breaking a offset calculation, which then turns negative. This raises the invalid times argument error, since times must be positive.

Should implement toSeqSnippet myself in the Modstrings package?

@lshep lshep assigned lshep and hpages and unassigned lshep Jan 17, 2020
@FelixErnst
Copy link
Contributor Author

Hi Herve @hpages

just wanted to bump this PR. Any comments?

I had to investigate some other encoding issue in BiocPkgTools package and by accident got a bit more insight intos the underlying issue.

  1. the compact ellipsis has a different raw value on windows systems. Therefore rawToChar(as.raw(c(0xe2, 0x80, 0xa6))) isn't working as expected.
  2. strsplit(xi,"")[[1L]] preserves the boundary of individual characters, whereas safeExplode(xi) relies on the correct lenght of the character vector being returned by LENGTH or XLENGTH in the C code. What I didn't realize initially is that even on Linux safeExplode(xi) destroys the ellipsis character. However, paste0 can stich it back together on Linux, which doesn't work on Windows.

Felix

@hpages
Copy link
Contributor

hpages commented Mar 9, 2020

Thanks Felix. I'm no longer using the compact ellipsis (see commit 567d8c3). Yes it saves 2 characters but it's hard to see so I'm back using the 3 dots.

@hpages hpages closed this Mar 9, 2020
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.

None yet

3 participants