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

Fix separator inside print #2820

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix separator inside print #2820

wants to merge 1 commit into from

Conversation

xaerru
Copy link

@xaerru xaerru commented Jan 18, 2025

Fix #2677 and #2668.

I think global_sep_space isn't needed because we set the sep to " " as the default when it's not specified.

I couldn't figure out why that if condition of two consecutive non characters was in place and also why sep_no_space was used.

When the if condition of two consecutive non characters was false, the empty separator was being pushed.

Copy link
Contributor

@kmr-srbh kmr-srbh left a comment

Choose a reason for hiding this comment

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

Thanks for the changes @xaerru! LPython is a sister project of a modern Fortran compiler LFortran. In Fortran, we do not add a space between 2 strings implicitly. We add a space only when either the left value or right value is something other than a string. The deleted code handled the separator based on this criteria.

Please see the discussion at https://github.com/lcompilers/lpython/pull/2670/files#r1581397608 on suggestion to fix this.

@xaerru
Copy link
Author

xaerru commented Jan 19, 2025

In LFortran ASR, Print node was refactored to only contain StringFormat or a single StringConstant.

Also for StringFormat, only FormatFortran is implemented as of now.

lfortran/lfortran#2543

lfortran/lfortran#4696

I think this should be done after libasr in LPython is synced to LFortran.

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: sep is ignored while printing when two adjacent values are either strings or integers
2 participants