moodymudskipper / flow

View and Browse Code Using Flow Diagrams
https://moodymudskipper.github.io/flow/
Other
403 stars 27 forks source link

PDF output has box character instead of spaces #70

Closed samueldnj closed 3 years ago

samueldnj commented 3 years ago

Hi,

This is a very useful package. I'm doing a code review with it this week.

When I use a pdf output from flow_view() or an html output from flow_doc(), the spaces are replaced with a box character. For example, if I run the following code

fileName <- "flowchart.pdf" flow_view( functionName, out = fileName)

I get this output:

optCPU.pdf

Thanks Sam

moodymudskipper commented 3 years ago

Hi Sam and thanks!

Could you give me your system info? And what software do you use to open the pdf?

I have replace regular spaces with an alternate space character to make it work with nomnoml. I believe the issue was that nomnoml will trim leading spaces so I had to do this to indent the code.

It's in : https://github.com/moodymudskipper/flow/blob/master/R/01_0_flow_data.R

The offending line is :

data$nodes$code_str <- gsub(" ", "\u2800", data$nodes$code_str)

I assumed it would work fine in all systems but it seems I was wrong. You might try to open the pdf with several readers or Web browsers, maybe your default one doesn't like this character for some reason.

samueldnj commented 3 years ago

Hi Moody,

Here's the output from sessionInfo():

R version 4.0.2 (2020-06-22) Platform: x86_64-apple-darwin17.0 (64-bit) Running under: macOS Catalina 10.15.7

I was opening the pdfs in Preview, but I also just used Firefox and an Adobe product, and the character is still there.

Thanks for the detailed explanation, I guess I could always edit a clone of the repo and install from local to adjust the character myself :)

Cheers! Sam

moodymudskipper commented 3 years ago

You might but you'd lose the indentation, it's not ideal. You could try to output to html and replace the character in the html file, maybe that would work, or to output to png if it's acceptable.

Meanwhile I'll keep an eye open for a solution, there might be another space character that is supported natively on mac too (I develop on Windows).

samueldnj commented 3 years ago

Thanks Antoine. I was hoping to include these in appendices for a code review contract I’m completing - complete with citation of course - so I might try to find a solution that works and suggest a pull request :)

Sam

On May 5, 2021, at 1:18 PM, Antoine Fabri @.***> wrote:

You might but you'd lose the indentation, it's not ideal. You could try to output to html and replace the character in the html file, maybe that would work, or to output to png if it's acceptable.

Meanwhile I'll keep an eye open for a solution, there might be another space character that is supported natively on mac too (I develop on Windows).

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/moodymudskipper/flow/issues/70#issuecomment-832979084, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABQ2ZRZFHED4PXMBYGB3QP3TMGRYXANCNFSM44DNDV5Q.

samueldnj commented 3 years ago

I'm pretty novice with Unicode (I made some ASCII art back in the day) but I want to confirm something before I start trying alternatives. There seem to be a few options for unicode space characters, but you're saying that the spaces are trimmed by nomnoml by default?

For example, the \u0020 character is equivalent to a regular space, so it would be trimmed?

Thanks!

moodymudskipper commented 3 years ago

I've just tried all possible space characters and nothing worked, you could try this as a compromise :

tmp <- flow:::flow_data
body(tmp)[[c(16, 3, 3)]] <- "\u00b7"
ns <- asNamespace("flow")
unlockBinding("flow_data", ns)
assignInNamespace("flow_data", tmp, ns)

(or fork and replace "\u2800" by "\u00b7")

image

Other usable characters would be "\u0387" and "\u2423" but look less good in my opinion.

"\2800" is a braille character, it's blank because it represent the 9 empty dots, the clean solution would be to get your system to recognize those, and tbh I have no idea how to do that :).

I'm glad that my package is useful to you, I hope we can figure something out.

moodymudskipper commented 3 years ago

Also, have you tried the plantuml engine (`engine = "plantuml") ? It doesn't use the same trick (it uses other unicode tricks!) so it might print better.

samueldnj commented 3 years ago

Thanks for trying Antoine, I really appreciate it!

I did use the plantuml engine, but for a couple of functions it cuts off printing a little soon. Some of these functions are very big.

On May 5, 2021, at 2:25 PM, Antoine Fabri @.***> wrote:

Also, have you tried the plantuml engine (`engine = "plantuml") ? It doesn't use the same trick (it uses other unicode tricks!) so it might print better.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/moodymudskipper/flow/issues/70#issuecomment-833017694, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABQ2ZR32GLSZ6BZGXZYASS3TMGZVDANCNFSM44DNDV5Q.

moodymudskipper commented 3 years ago

Leading spaces being removed is a limitation as discussed above BUT multiple spaces are removed only when using svg = TRUE in flow_view_nomnoml() which happens when plotting to the viewer window, but not when plotting to png for instance. this means we could avoid these box characters by using svg = FALSE and providing a prefix, so we could have something like the following :

image

This could be set through an option to avoid this issue.

options(flow_line_prefix = "> ") would reproduce the above.

We'd lose the svg output, which only means in practice that we can't select/copy the code from the diagrams, which is not all that useful.

This option should be documented

moodymudskipper commented 3 years ago

@samueldnj FYI though it might not be useful for you anymore :

You can now use options(flow.indenter = "\u00b7") to avoid having box characters (or use any character you like better). Basically double spaces are replaced by this character followed by a regular space. By default this character is the Braille character that was causing you issues.

In practice I believe we'll see this character only in indentation and when multiple subsequent spaces are used in quoted strings.

samueldnj commented 3 years ago

Thanks Antoine, appreciated!

On Aug 10, 2021, at 3:40 PM, Antoine Fabri @.***> wrote:

@samueldnj https://github.com/samueldnj FYI though it might not be useful for you anymore :

You can now use options(flow.indenter = "\u00b7") to avoid having box characters (or use any character you like better). Basically double spaces are replaced by this character followed by a regular space. By default this character is the Braille character that was causing you issues.

In practice I believe we'll see this character only in indentation and when multiple subsequent spaces are used in quoted strings.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/moodymudskipper/flow/issues/70#issuecomment-896358254, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABQ2ZR45FEFVH3DBVD3WLF3T4GTFFANCNFSM44DNDV5Q. Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email.

github-actions[bot] commented 2 years ago

This old thread has been automatically locked. If you think you have found something related to this, please open a new issue and link to this old issue if necessary.