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

Avoid progress bar overlap #1866

Merged
merged 8 commits into from
Aug 1, 2024
Merged

Conversation

TwoOfTwelve
Copy link
Contributor

@TwoOfTwelve TwoOfTwelve commented Jul 11, 2024

Avoids overlaps of the progress bar with log outputs by delaying the log outputs while a progress bar is being shown.

Log outputs that happen during the progress bar are instead shown below it, after it is done:

Loading Submissions   100% [=========================] 2/2 (0:00:00 / 0:00:00) 
Parsing Submissions     0% [                               ] 0/2 (0:00:00 / ?) line 5:12 mismatched input '=' expecting '{'
line 5:12 mismatched input '=' expecting '{'
Parsing Submissions   100% [=========================] 2/2 (0:00:00 / 0:00:00) 
2024-07-11-12:01:45_008 [WARN] Submission - Failed to parse submission test1.cs:
failed to parse 'test1.cs'class de.jplag.csharp.grammar.CSharpParser$Namespace_or_type_nameContext cannot be cast to class de.jplag.csharp.grammar.CSharpParser$Local_variable_declarationContext (de.jplag.csharp.grammar.CSharpParser$Namespace_or_type_nameContext and de.jplag.csharp.grammar.CSharpParser$Local_variable_declarationContext are in unnamed module of loader 'app')
2024-07-11-12:01:45_012 [WARN] Submission - Failed to parse submission test2.cs:
failed to parse 'test2.cs'class de.jplag.csharp.grammar.CSharpParser$Namespace_or_type_nameContext cannot be cast to class de.jplag.csharp.grammar.CSharpParser$Local_variable_declarationContext (de.jplag.csharp.grammar.CSharpParser$Namespace_or_type_nameContext and de.jplag.csharp.grammar.CSharpParser$Local_variable_declarationContext are in unnamed module of loader 'app')

There is still some overlap with antlr, because antlr writes directly to System.err. That problem is addressed in #1868.

@tsaglam tsaglam added bug Issue/PR that involves a bug minor Minor issue/feature/contribution/change labels Jul 11, 2024
@tsaglam tsaglam added this to the 6.0.0 milestone Jul 11, 2024
@TwoOfTwelve TwoOfTwelve requested review from a team and removed request for tsaglam July 18, 2024 11:35
Copy link
Member

@tsaglam tsaglam left a comment

Choose a reason for hiding this comment

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

There are a few issues regarding sync and naming here. Also, please check the Sonar issues.

@Kr0nox
Copy link
Member

Kr0nox commented Jul 19, 2024

grafik
Do we want the errors to be printed like they are in the screenshot or after a specific progress bar has run to 100%.
With the current way, a specific progress bar may be printed multiple times with different percentages.

@tsaglam
Copy link
Member

tsaglam commented Jul 22, 2024

Do we want the errors to be printed like they are in the screenshot or after a specific progress bar has run to 100%. With the current way, a specific progress bar may be printed multiple times with different percentages.

Both would be possible. Without the duplication, it looks cleaner. With the duplication, you get feedback earlier. For large datasets, this allows you to already check out the erroneous submissions while JPlag is still running.

@jplag/hiwi @jplag/maintainer any preferences?

@Kr0nox
Copy link
Member

Kr0nox commented Jul 22, 2024

I think printing them after (and thus having no duplication) would be better.

  1. Then all errors are accumulated together and not split between multiple instances of the progress bar
  2. It is easier to follow the process of the current taks, when you dont have to switch progress bars.

But that is just my opinion. Feel free to object

@Kr0nox
Copy link
Member

Kr0nox commented Jul 22, 2024

grafik
There also seems to be an issue with printing. These two failed submission are exact copies of each other, but the reason is only printed for one of them

@TwoOfTwelve
Copy link
Contributor Author

Can you try again. Now there should be nothing cut of.

Copy link

sonarcloud bot commented Jul 26, 2024

@Kr0nox
Copy link
Member

Kr0nox commented Jul 30, 2024

There also seems to be an issue with printing. These two failed submission are exact copies of each other, but the reason is only printed for one of them

This seems to be fixed now

Do we want the errors to be printed like they are in the screenshot or after a specific progress bar has run to 100%.
With the current way, a specific progress bar may be printed multiple times with different percentages.

@TwoOfTwelve Did you also change it to this behavior? Or was it the default in your PR anyways or is my dataset just small enough that the errors are printed so late?

@TwoOfTwelve
Copy link
Contributor Author

With the way it's implemented in this PR the error will always be printed after the progress bar has reach 100% or JPlag was aborted. I could try to change that behavior by interrupting the progress bar when an error occurs, but that would be a lot more work.

@tsaglam tsaglam merged commit eb6dca8 into develop Aug 1, 2024
44 checks passed
@tsaglam tsaglam deleted the feature/avoidProgressBarOverlap branch August 1, 2024 11:18
@tsaglam tsaglam linked an issue Aug 2, 2024 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue/PR that involves a bug minor Minor issue/feature/contribution/change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Progessbars cut off error messages
3 participants