Skip to content

Incorrect line number display of process logs in actions #23680

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

Closed
yp05327 opened this issue Mar 24, 2023 · 7 comments · Fixed by #23820
Closed

Incorrect line number display of process logs in actions #23680

yp05327 opened this issue Mar 24, 2023 · 7 comments · Fixed by #23820
Labels
topic/gitea-actions related to the actions of Gitea type/bug
Milestone

Comments

@yp05327
Copy link
Contributor

yp05327 commented Mar 24, 2023

Description

https://gitea.com/yp05327/testrepo/actions/runs/14
image

Gitea Version

1.20.0+dev-251-g9f39def99

Can you reproduce the bug on the Gitea demo site?

Yes

Log Gist

No response

Screenshots

No response

Git Version

No response

Operating System

No response

How are you running Gitea?

gitea.com

Database

None

@yp05327 yp05327 changed the title Incorrect line number display in actions Incorrect line number display of process logs in actions Mar 24, 2023
@yp05327
Copy link
Contributor Author

yp05327 commented Mar 24, 2023

It seems that the process state icon's location is also strange.
image

@a1012112796 a1012112796 added the topic/gitea-actions related to the actions of Gitea label Mar 25, 2023
@lunny lunny added this to the 1.19.1 milestone Mar 30, 2023
@wxiaoguang
Copy link
Contributor

wxiaoguang commented Mar 30, 2023

The line number is correct. However, some CLI programs use "\r" to print new content in current line.

So, the strings in "line 1" are actually from \rReading...1%\rReading...5%\rReading...100%\n

Two approaches to make it better:

  1. Treat all "\r" as "\n"
  2. Gitea assembles the content, only renders the last "Reading...100%\n".

Personally, I dislike 1, prefer 2. Because, approach 1 would render a lot of non-sense "progress" information.

Approach 2 renders what end users would see in the end.

A demo program:

#include <stdio.h>
#include <unistd.h>

int main()
{
    printf("testing ...\n");
    for (int i = 0; i < 10; i++) {
        printf("\r%d", i);
        fflush(stdout);
        sleep(1);
    }
    return 0;
}

@wxiaoguang
Copy link
Contributor

I guess #23820 could help (I haven't tried). Could you take a look? Thank you.

@silverwind
Copy link
Member

silverwind commented Mar 30, 2023

Gitea assembles the content, only renders the last "Reading...100%\n".

Also prefer this if line deletions can be reliably detected and removed, e.g. just render it like a terminal would after modifications are completed on the line.

@silverwind
Copy link
Member

It seems that the process state icon's location is also strange. image

I think #23789 has fixed that alignment.

@yp05327
Copy link
Contributor Author

yp05327 commented Mar 31, 2023

I think #23789 has fixed that alignment.

Yes, it fixed in #23789

@lunny lunny closed this as completed Mar 31, 2023
@wxiaoguang
Copy link
Contributor

wxiaoguang commented Mar 31, 2023

Wait, keep it open.

The key problem (line number) hasn't been fixed. #23789 only improves some alignments.

@lunny lunny reopened this Mar 31, 2023
lunny pushed a commit that referenced this issue Apr 1, 2023
Close #23680

Some CLI programs use "\r" and control chars to print new content in
current line.

So, the strings in one line are actually from
`\rReading...1%\rReading...5%\rReading...100%`

This PR tries to make the output better.
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
topic/gitea-actions related to the actions of Gitea type/bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants