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

IO.print without new line #10858

Merged
merged 8 commits into from
Aug 24, 2024
Merged

IO.print without new line #10858

merged 8 commits into from
Aug 24, 2024

Conversation

JaroslavTulach
Copy link
Member

@JaroslavTulach JaroslavTulach commented Aug 21, 2024

Pull Request Description

Fixes #10028 by adding IO.print function that doesn't add new line at the end of its message.

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

  • The documentation has been updated, if necessary.
  • All code follows the
    Scala,
    Java,
    TypeScript,
    and
    Rust
    style guides. In case you are using a language not listed above, follow the Rust style guide.
  • Unit tests have been written where possible.

println : Any -> Nothing
println message = @Builtin_Method "IO.println"
println : Any -> Text -> Nothing
println message nl='\n' = IO_Helpers.println message nl
Copy link
Member

Choose a reason for hiding this comment

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

Please let's not use such shorthands in our public (1) API. I'd suggest to call this argument newline or we could do similar to Python: end or perhaps a more explicit end_with = '\n' (I think I like end_with the most, but that's just subjective).

Also the documentation should be updated to include the added argument in the Arguments: section.

(1) The PRIVATE comment at the top may be confusing. It is used there to prevent a text-oriented method from showing up in the GUI's component browser. But the println method is definitely part of our 'official' API and so it should be well documented for end-users. As already mentioned we probably should create a separate tag, like TEXT_ONLY or HIDDEN, for documenting components that are not internal but should not show up in the GUI Component Browser.

@farmaazon
Copy link
Contributor

My twopence: if println does not end necessarily with newline, it should be called print (as it is in python).

@JaroslavTulach JaroslavTulach force-pushed the wip/jtulach/Print10028 branch from 36699e9 to f089030 Compare August 21, 2024 13:29
@Akirathan
Copy link
Member

My twopence: if println does not end necessarily with newline, it should be called print (as it is in python).

I agree. Why not have two methods - IO.println and IO.print just like in dozens of other languages?

@radeusgd
Copy link
Member

My twopence: if println does not end necessarily with newline, it should be called print (as it is in python).

Fair point. I'd suggest that we keep println as-is (without adding any new optional arguments) and then add print variant that does not append any newlines (again it would not need additional arguments).

This way we can keep using println as-is - as we are using it with the default newline in 99% of the codebase; and in places where we need no newline we can use print instead (which is currently just a single place).

IMO separating into two functions would be clearest. And it's already done that way e.g. in Java, so people will be familiar with it.

@4e6
Copy link
Contributor

4e6 commented Aug 22, 2024

I agree with Adam. IMO the function should've been called print from the beginning.

@JaroslavTulach
Copy link
Member Author

I agree with Adam. IMO the function should've been called print from the beginning.

The problem with Adam's rename to print is that such a change is backward incompatible. Easier to just

we keep println as-is (without adding any new optional arguments) and then add print variant that does not append any newlines (again it would not need additional arguments).

As proposed by @radeusgd and voted up by @GregoryTravis, @hubertp and @Akirathan. I'll prepare the patch.

@JaroslavTulach JaroslavTulach changed the title IO.println without new line IO.print without new line Aug 22, 2024
@JaroslavTulach JaroslavTulach added the CI: Clean build required CI runners will be cleaned before and after this PR is built. label Aug 23, 2024
@JaroslavTulach
Copy link
Member Author

JaroslavTulach commented Aug 23, 2024

CRLF failure on Windows will be fixed by 7e8feee

@JaroslavTulach JaroslavTulach merged commit b6f0675 into develop Aug 24, 2024
41 checks passed
@JaroslavTulach JaroslavTulach deleted the wip/jtulach/Print10028 branch August 24, 2024 05:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: Clean build required CI runners will be cleaned before and after this PR is built.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Don't use System.out.print in Standard.Test - use IO!
6 participants