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

Web: handle query string input line by line #206

Merged
merged 1 commit into from
Oct 23, 2023

Conversation

eminence
Copy link
Contributor

This changes how input from a query string are handled. Handling them line by line produces output that more closely resembles how it would have looked like when typed in interactively.

Note that this is technically a breaking change: existing numbat.dev URLs in the wild that contain newlines in the middle of statements will no longer work.

Closes #205

@sharkdp
Copy link
Owner

sharkdp commented Oct 11, 2023

Thank you very much. It handles multi-line inputs like

@aliases(points)
unit point

correctly because those can be combined into a single line (@aliases(points) unit point). But it does not work for multi-statement inputs like

let x=1
let y=2

because the combination let x=1 let y=2 is not valid syntax.

This is why I previously opted for the "combine everything into one block" solution.

I want to introduce semicolons eventually, but those wouldn't help either because we do not want to introduce a semicolon to join lines in the first example above. We would need help from the parser to know where to potentially split.

What might work is to have some special symbol just for encoding inputs in the URL. Maybe something like (U+23CE). Then we could keep \n newlines in the user input, but join multiple inputs with the special symbol. When loading, we would simply split on instead of \n.

@eminence
Copy link
Contributor Author

Ahh, yes, this doesn't handle that type of multi-line input. I'll implement your idea with , but I'm now wondering if what we really need to do is encode a JSON list in the query string.

Though I guess it's worth clarifying if you think the issue I reported in #205 is actually an issue worth fixing. I think maybe someone could argue successfully that the current behavior is desired (or maybe even preferred, because it produces a more compact output)

@sharkdp
Copy link
Owner

sharkdp commented Oct 18, 2023

Though I guess it's worth clarifying if you think the issue I reported in #205 is actually an issue worth fixing.

Yes, absolutely!

I think maybe someone could argue successfully that the current behavior is desired (or maybe even preferred, because it produces a more compact output)

On the other hand, we are currently hiding some results. So yours is the better option, in my opionion.

This changes how input from a query string are handled.  Handling them
line by line produces output that more closely resembles how it would
have looked like when typed in interactively.

Closes sharkdp#205
@eminence
Copy link
Contributor Author

Sorry for the delay here. I've rebased and pushed a commit that adopts the approach you suggested above.

Copy link
Owner

@sharkdp sharkdp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you

@sharkdp sharkdp merged commit 17d3e69 into sharkdp:master Oct 23, 2023
14 checks passed
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.

Small problem in web calculator rendering
2 participants