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

Display error in GitLab pipeline log #314

Closed
escalate opened this issue Jan 20, 2023 · 10 comments · Fixed by #378
Closed

Display error in GitLab pipeline log #314

escalate opened this issue Jan 20, 2023 · 10 comments · Fixed by #378
Labels
bug Something isn't working

Comments

@escalate
Copy link

escalate commented Jan 20, 2023

Describe the bug
In the pipeline log of the GitLab web interface, characters of the log output of multi-gitter status are truncated.

Log output in the web interface:

ttps://gitlabserver.de/infrastructure/base-images/php/-/merge_requests/51�base-images/php #51uccess

Raw log output:

�]8;;https://gitlabserver.de/infrastructure/base-images/php/-/merge_requests/51�base-images/php #51�]8;;�: Success

To Reproduce

  1. Create MRs with multi-gitter run --platform="gitlab" ...
  2. Display status of MRs with multi-gitter status --platform="gitlab" ...
  3. Watch log output in GitLab web interface

Expected behavior
The log output is displayed completely, legibly and optimized for display in a CI.

Additional context
The problem seams to be in the file https://github.com/lindell/multi-gitter/blob/master/internal/multigitter/terminal/terminal.go
I think it is not a good idea to use the special URL syntax, because it is probably not understood by any CI.

@escalate escalate added the bug Something isn't working label Jan 20, 2023
@lindell lindell closed this as completed Jan 20, 2023
@lindell lindell reopened this Jan 20, 2023
@lindell
Copy link
Owner

lindell commented Jan 20, 2023

Hello.

It seems that the GitLab CI view has not implemented terminal output correctly, but some things (like color seems to be supported).

From the links spec:

Any terminal that correctly implements OSC parsing according to ECMA-48 is guaranteed not to suffer from compatibility issues. That is, even if explicit hyperlinks aren't supported, the target URI is silently ignored and the supposed-to-be-visible text is displayed, without artifacts.

If a terminal emits garbage upon an OSC 8 explicit hyperlink sequence, that terminal is buggy according to ECMA-48.

https://gist.github.com/egmontkob/eb114294efbcd5adb1944c9f3cb5feda


However, I think it is a good idea to support non formatted output, but under a flag (or if the incompatibility could be detected), under a flag like --plain-output or similar.

@escalate
Copy link
Author

Hello @lindell,
is this new feature something you plan to develop in the near future?
Best regards
Felix

@lindell
Copy link
Owner

lindell commented Jan 28, 2023

This is definitely high on the list to do. But I currently don't have a lot of time to spend on coding. But if this is something you need, I'm happy to merge a PR with the plain flag implemented.

@lindell
Copy link
Owner

lindell commented Jul 17, 2023

#378 should fix this. Please test and verify

It can be installed with go install github.com/lindell/multi-gitter@plain-output

@lindell
Copy link
Owner

lindell commented Jul 30, 2023

@escalate ?

@escalate
Copy link
Author

escalate commented Jul 31, 2023

Hello @lindell,

unfortunately I was on vacation abroad and I am available again today.
The changes look good so far.
I would like to test them this week and give feedback.

Best regards
Felix

@escalate
Copy link
Author

escalate commented Aug 4, 2023

Hello @lindell

I have tested all commands (run, status merge, close, print).
There were no changes in the output of run, merge, close and print.
Status is now output correctly without the special link syntax.

However, I have discovered one more issue that is now visible with the new option.
Sometimes Status does not print the full path to the git repository.
I don't understand exactly when it happens.
Here is an example:

# used option
--group="systems" --include-subgroups \
# normal output
�]8;;https://gitlabserver.de/systems/next-js-workshop/-/merge_requests/2�systems/next-js-workshop #2�]8;;�: Success
# plain output
systems/nest-js-workshop #5: Success
# expected path
systems/next-js-workshop
# used option
--group="products" --include-subgroups \
# normal output
�]8;;https://gitlabserver.de/products/data-services/statistik-portal/-/merge_requests/580�data-services/statistik-portal #580�]8;;�: Success
# plain output
data-services/statistik-portal #583: Success
# expected path
products/data-services/statistik-portal

Could you please take a look at this phenomenon?

Best regards
Felix

@lindell
Copy link
Owner

lindell commented Oct 2, 2023

I have tested all commands (run, status merge, close, print).
There were no changes in the output of run, merge, close and print.

I've been trying to understand why it does not work in run, merge, close and print. It works in the test and when I'm trying it locally.

The second one was a simple bug that I have now solved in #392

@escalate
Copy link
Author

escalate commented Oct 8, 2023

Hello @lindell.

You have misunderstood me here.
The bug was about the sub command "status".
With your last fix of the path everything is correct now.

I have also tested the other sub commands for completeness.
There were no differences to the previous version.
I would rate this neutrally.

There are no further ToDos.
The issue can be closed.

Best regards
Felix

@github-actions
Copy link
Contributor

Included in release v0.48.0 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants