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

Consider replacing newlines with empty space in text visualizer #1670

Closed
glopesdev opened this issue Feb 3, 2024 · 6 comments · Fixed by #1765
Closed

Consider replacing newlines with empty space in text visualizer #1670

glopesdev opened this issue Feb 3, 2024 · 6 comments · Fixed by #1765
Labels
proposal Request for a new feature
Milestone

Comments

@glopesdev
Copy link
Member

The new text visualizer now strips line endings with empty strings so each value fits in a single row of the buffer. An easy way to preserve some extra readability would be to replace the line endings with a single white space.

@glopesdev glopesdev added the proposal Request for a new feature label Feb 3, 2024
@glopesdev glopesdev added this to the 2.9 milestone Feb 3, 2024
@glopesdev glopesdev modified the milestones: 2.9, 2.8.3 Apr 15, 2024
@glopesdev
Copy link
Member Author

Another option would be to replace all hidden control characters with a rendition of the respective control character, e.g. https://en.wikipedia.org/wiki/Unicode_control_characters#Control_pictures

@PathogenDavid
Copy link
Member

I did a partial implementation of using control picture replacements. While it does work, legibility with the default fonts on Windows 10 is not ideal:

image

Not sure if it maybe looks better on Windows 11. (During dev club I noticed the control pictures did look different on your screen.) If you want to check here's my WIP branch and my test file.

I did notice that this visualizer uses RichTextBox, so in theory we could dim the glyphs which would probably help. That might complicate things for non-winforms UI frontends though and might be overkill.

Assuming it even matters here, it could also put us down a bad path performance-wise. (I didn't dig super far but RichTextBox does seem to have a completely separate path for handling plain text.)

@glopesdev
Copy link
Member Author

@PathogenDavid good point, we've had issues with high-frequency streams before. We now buffer and batch updates automatically so the impact should be less pronounced but would be nice to have a performant code path by default.

I guess another option might be to just replace the control characters by their literal c# escape characters, i.e. tab with \t, newline with \n, etc.

@bruno-f-cruz any thoughts or feedback on this?

@bruno-f-cruz
Copy link
Contributor

I have mixed feelings:

As a developer, I like the change as it would offer a more powerful way to inspect the data streams.

As someone who interacts with new users frequently, and given how steep the learning curve of bonsai is perceived to be, I am afraid that the proposed change will add complexity to one (if not the) most core visualizer of the IDE. We must consider that many of the users of the IDE have no idea what a control character even is.

My proposal would be to have some sort of control in the text editor that toggles between the two modes. Alternatively, we could also think about ways to have "decorator"-like functionality to the text visualizer. It would be interesting to think how this could be extended to support other forms of richer text visualization (thinking about hex viewer, regexp text highlight, etc...)

@glopesdev
Copy link
Member Author

glopesdev commented Apr 25, 2024

@bruno-f-cruz Yup, I am a bit torn on this one too. The first time I remember hitting this was with matrix representations, either from Math.NET or OpenTK where they would actually include new lines in the ToString method result. For that one it is definitely nicer to simply flatten the rows with a regular space separator (since each row is bracketed anyway).

The other case which is more problematic is NetMQ messages where strings can be returned with a \0 character which literally just kills the control, since we are appending strings and even a single \0 will prevent any other row from being rendered. In that case it's not clear whether we want to replace \0 with a space, especially if it ends up being a dump of some kind of buffered character array from a serial port or network protocol.

PathogenDavid added a commit to PathogenDavid/bonsai that referenced this issue Apr 29, 2024
@PathogenDavid
Copy link
Member

PathogenDavid commented Apr 30, 2024

For the sake of posterity: We discussed this again earlier today during dev club and it was decided that the original proposal (replacing newlines with whitespace) is the ideal solution for now and later we will provide an "advanced" visualization via a package or something similar for those who know and care about control characters.


We didn't really talk about how we wanted to handle \0 in this case. In my current proposed implementation I opted to replace it (along with all the other unprintable control characters) with "�" the replacement character.

For reference here's what that looks like along with some additional new test strings:

image

Since I had it implemented on Friday and for the sake of comparison, here's what the C# escape sequences implementation would've looked like:

image

and for further comparison, here's control pictures with the new sample strings:

image

(The �s in this case are bring printed by Windows since those are unprintable C1 control characters and have no control pictures.)

For future reference, here are the prototypes pictured above: PathogenDavid@d9ac1d8

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal Request for a new feature
Projects
None yet
3 participants