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

Add space between multiple strings while printing #2670

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

kmr-srbh
Copy link
Contributor

fixes #2669

print("a", "b", "c", "d")

fruits: list[str] = ["apple", "guava", "mango"]
mango: str = fruits.pop()
print("I have got a", mango, "to eat!")
(base) saurabh-kumar@Awadh:~/Projects/System/lpython$ ./src/bin/lpython ./examples/example.py
a b c d
I have got a mango to eat!

@kmr-srbh
Copy link
Contributor Author

@Shaikh-Ubaid could you please rerun the CI? The failing check is unrelated to the PR.

@Shaikh-Ubaid
Copy link
Collaborator

The approach in this PR might break lfortran. lfortran needs to not print spaces when two consecutive print arguments are strings.

% cat examples/expr2.f90     
program expr2
implicit none

print *, "hi", "bye"
print *, 12, 23

end program
% gfortran examples/expr2.f90 
% ./a.out 
 hibye
          12          23
% lfortran examples/expr2.f90
hibye
12 23

if (global_sep_space &&
!(ASRUtils::is_character(*ASRUtils::expr_type(x.m_values[i]))
&& ASRUtils::is_character(*ASRUtils::expr_type(x.m_values[i - 1])))) {
if (global_sep_space) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

The way to fix this such that we print spaces when two consecutive arguments are strings in LPython and not print spaces for the same in LFortran, is to support this in AST->ASR Translation.

Copy link
Collaborator

Choose a reason for hiding this comment

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

You will need to figure out how to update the middle-ends of both lfortran and lpython such that both of them work as expected when two consecutive arguments are strings.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Shaikh-Ubaid thanks for this! I will try to create a solution that works for both. 👍

Copy link
Collaborator

Choose a reason for hiding this comment

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

See if your solution works for both lpython and lfortran (test it locally on your system or send a PR to lfortran so it gets tested at the CI). If no run reference tests change for lfortran, then it works.

You can also test your current approach with lfortran. If it works currently, then I think its good.

@Shaikh-Ubaid Shaikh-Ubaid marked this pull request as draft April 26, 2024 18:36
@kmr-srbh kmr-srbh force-pushed the add-space-between-strings-in-print branch from c52e653 to 6fc0b6a Compare April 27, 2024 05:06
@kmr-srbh kmr-srbh changed the title Add space between multiple strings in print Add space between multiple strings while printing Apr 29, 2024
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.

Bug: Printing multiple strings together does not add a space between them
2 participants