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

Implement some quality-of-life features #102

Merged
merged 13 commits into from
Jan 22, 2025

Conversation

Luro02
Copy link
Collaborator

@Luro02 Luro02 commented Jan 13, 2025

Color changes

The old code did not handle the light and dark theme properly, and the default highlighter color was unreadable in the light theme:
old_color_white_theme

This has been changed to a much more pleasant color:
new_white_theme

Similarly for the dark theme, the color has been adjusted to be more pleasant.

This is the before:
image

and this is what it was changed to

Screenshot 2025-01-13 085119

Internally an extra class called ThemeColor has been introduced. Currently it does the same things that JBColor does, but in the future, it will be changed to accommodate new features (like a nested annotation having a different highlighter color or buttons having different highlighter colors). This would not be so easy with a JBColor.

The old code did not support changing the transparency of the highlighter color, this is now supported.

Regarding migrations

With the types changed, loading an old config, would have crashed the plugin. The code has been extended to migrate old values to the new format. Additionally, it will migrate the old default highlighter color to the new one. The other colors are not migrated, because they haven't been changed.

Automatically selects the right exercise, based on the selected config

This PR will close #92.

When intelligrade was started, it loaded the config that had been selected previously and defaulted to the first exercise in the list.

Now one can select the config, and it will automatically select the first exercise that can be used with the config.

Save will cleanup the assessment

The code will now remove the local files and close the assessment after the save was pressed in this menu:
image

This removes the need to press the save button and then the close button.

Automatically open a file when there is no main method present

This PR will close #88.

The code does nothing fancy; it will select the first class that it encounters.

When a line is selected via double-click it highlights the following line too.

This PR will close #93. Turns out this was a misinterpretation of the offset. The end offset is exclusive in the text range. That resulted in the wrong selection when double-clicking a line.

In addition to that, I fixed some potential threading problems (according to the javadoc, some methods must be called in a read action).

Automatically set inspection profile

With this PR an inspection profile is automatically set in the cloned submission. This PR closes #91.

Add support for buttons without visible highlighting

It will now look like this for certain annotations:
image

Note that there is a pen on the left side, but the line is not highlighted. This is intentional.

@Luro02 Luro02 marked this pull request as ready for review January 13, 2025 15:53
@Luro02 Luro02 requested review from CDaut and a team as code owners January 13, 2025 15:53
@Luro02 Luro02 mentioned this pull request Jan 16, 2025
3 tasks
@Feuermagier
Copy link
Collaborator

I'm generally fine with the PR, though I'm not sure if it's really worth it to replace JBColor? Why can't we simply wrap it in the future situations that you describe?

@Luro02
Copy link
Collaborator Author

Luro02 commented Jan 16, 2025

I'm generally fine with the PR, though I'm not sure if it's really worth it to replace JBColor? Why can't we simply wrap it in the future situations that you describe?

The initial implementation wanted to do just that, but one problem is that you can only obtain the separate dark/light color through the api internal methods. There is another way, but the whole thing is not trivial, especially because the returned color changes depending on the active theme.

Sure, we could use the internal api, but this can lead to things breaking unnecessarily in the future or maybe not, depends on the intellij developers. Another concern was, like mentioned, that I am planning to extend this type with more data in the future, like for example:

  • we could change the highlighting color depending on the button, instead of hardcoding it, we might pass a name for it to Intelligrade like "green"
  • maybe change the way code is highlighted for some things. If I understood the code correctly, it is possible to change the style from changing the background color to maybe a colored underline in the code.

The biggest reason for making a dedicated type was that migrating the contig is a pain, and I don't want to have tons of legacy code for old settings options. Deleting the config is not trivial either without our current structure.

By having a dedicated class, future config changes won't necessarily result in a class change.


To be honest the arguments are all kinda weak, especially with me not having the old code anymore. The lines of code aren't much different between the two implementations and not using the internal apis seemed like a nice plus. So I would much rather keep that class in, it makes future changes a bit less annoying.

@dfuchss dfuchss requested a review from SoftHeinrich January 17, 2025 12:17
@Feuermagier
Copy link
Collaborator

Ok, I'm fine with it if you think it's better this way

Copy link

@SoftHeinrich SoftHeinrich left a comment

Choose a reason for hiding this comment

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

better readability!

@Luro02 Luro02 merged commit 7fdac5c into kit-sdq:main Jan 22, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants