Adds macOS theme switch listening

[0verEngineer]

#408

[Murmele]

@0verEngineer thanks for the work and @LenaWil thanks for the review!

@0verEngineer can you remove the ifdef for testing. Then we are getting builds for windows, linux and macos and we can test it easily.

@LenaWil did it work for you?

[0verEngineer]

@Murmele I have removed the ifdefs and i also tested the event thing on Gnome but no event is fired on theme neither lagacy theme switch.

[0verEngineer]

Checked KDE and Windows Theme change in Settings:

KDE: ThemeChange and StyleChange event triggered

Windows: ThemeChange, StyleChange and PaletteChange events triggered multiple times

If we reload the theme on each of the 3 Events it will be done multiple times in a row, idk how this works performance wise, i think we should reload on Windows and KDE only on the ThemeChange event and on MacOS like it is now.

I will do it later :)

[LenaWil]

@LenaWil did it work for you?

I haven’t actually set up a build-environment currently, before I do that, is there a way to easily downloaded a pre-built version?

[Murmele]

@LenaWil did it work for you?

I haven’t actually set up a build-environment currently, before I do that, is there a way to easily downloaded a pre-built version?

Yes, just check the build artifacts. There you can find a macos build.

https://github.com/Murmele/Gittyup/actions/runs/3970480332

[Murmele]

Checked KDE and Windows Theme change in Settings:

KDE: ThemeChange and StyleChange event triggered

Windows: ThemeChange, StyleChange and PaletteChange events triggered multiple times

If we reload the theme on each of the 3 Events it will be done multiple times in a row, idk how this works performance wise, i think we should reload on Windows and KDE only on the ThemeChange event and on MacOS like it is now.

I will do it later :)

Thanks for testing. Would be nice if we can fix it on all three OS.

[LenaWil]

@LenaWil did it work for you?

Okay definitely better than first, the only thing is that the code preview doesn’t update properly and still has the dark/light theme after switching to the opposite. But I can at least read the code now!

[Murmele]

great! Something for scintilla is missing

[Murmele]

Maybe we have to implement the event also for the texteditor

TextEditor::TextEditor(QWidget *parent) : ScintillaIFace(parent) {
  // Load colors.
  Theme *theme = Application::theme();
  mOursColor = theme->diff(Theme::Diff::Ours);
  mTheirsColor = theme->diff(Theme::Diff::Theirs);
  mAdditionColor = theme->diff(Theme::Diff::Addition);
  mDeletionColor = theme->diff(Theme::Diff::Deletion);

There are more theme related things below

// Set word diff indicators.
  indicSetStyle(WordAddition, INDIC_STRAIGHTBOX);
  indicSetFore(WordAddition, theme->diff(Theme::Diff::WordAddition));
  indicSetAlpha(WordAddition, 255);
  indicSetUnder(WordAddition, true);

  indicSetStyle(WordDeletion, INDIC_STRAIGHTBOX);
  indicSetFore(WordDeletion, theme->diff(Theme::Diff::WordDeletion));
  indicSetAlpha(WordDeletion, 255);
  indicSetUnder(WordDeletion, true);

  indicSetStyle(NoteIndicator, INDIC_SQUIGGLE);
  indicSetFore(NoteIndicator, theme->diff(Theme::Diff::Note));
  indicSetAlpha(NoteIndicator, 255);
  indicSetUnder(NoteIndicator, true);

  indicSetStyle(WarningIndicator, INDIC_STRAIGHTBOX);
  indicSetFore(WarningIndicator, theme->diff(Theme::Diff::Warning));
  indicSetAlpha(WarningIndicator, 255);
  indicSetUnder(WarningIndicator, true);

  indicSetStyle(ErrorIndicator, INDIC_STRAIGHTBOX);
  indicSetFore(ErrorIndicator, theme->diff(Theme::Diff::Error));
  indicSetAlpha(ErrorIndicator, 255);
  indicSetUnder(ErrorIndicator, true);

  // Initialize LPeg lexer.
  QColor base = palette().color(QPalette::Base);
  QColor text = palette().color(QPalette::Text);
  bool dark = (text.lightnessF() > base.lightnessF());

  setLexerLanguage("lpeg");
  setProperty("lexer.lpeg.home", Settings::lexerDir().path());
  setProperty("lexer.lpeg.themes", theme->dir().path());
  setProperty("lexer.lpeg.theme", theme->name());
  setProperty("lexer.lpeg.theme.mode", dark ? "dark" : "light");

moving all related code to the end of the constructor if this is working in own function which will be called also for those events

[0verEngineer]

Maybe we have to implement the event also for the texteditor

TextEditor::TextEditor(QWidget *parent) : ScintillaIFace(parent) {
  // Load colors.
  Theme *theme = Application::theme();
  mOursColor = theme->diff(Theme::Diff::Ours);
  mTheirsColor = theme->diff(Theme::Diff::Theirs);
  mAdditionColor = theme->diff(Theme::Diff::Addition);
  mDeletionColor = theme->diff(Theme::Diff::Deletion);

There are more theme related things below

// Set word diff indicators.
  indicSetStyle(WordAddition, INDIC_STRAIGHTBOX);
  indicSetFore(WordAddition, theme->diff(Theme::Diff::WordAddition));
  indicSetAlpha(WordAddition, 255);
  indicSetUnder(WordAddition, true);

  indicSetStyle(WordDeletion, INDIC_STRAIGHTBOX);
  indicSetFore(WordDeletion, theme->diff(Theme::Diff::WordDeletion));
  indicSetAlpha(WordDeletion, 255);
  indicSetUnder(WordDeletion, true);

  indicSetStyle(NoteIndicator, INDIC_SQUIGGLE);
  indicSetFore(NoteIndicator, theme->diff(Theme::Diff::Note));
  indicSetAlpha(NoteIndicator, 255);
  indicSetUnder(NoteIndicator, true);

  indicSetStyle(WarningIndicator, INDIC_STRAIGHTBOX);
  indicSetFore(WarningIndicator, theme->diff(Theme::Diff::Warning));
  indicSetAlpha(WarningIndicator, 255);
  indicSetUnder(WarningIndicator, true);

  indicSetStyle(ErrorIndicator, INDIC_STRAIGHTBOX);
  indicSetFore(ErrorIndicator, theme->diff(Theme::Diff::Error));
  indicSetAlpha(ErrorIndicator, 255);
  indicSetUnder(ErrorIndicator, true);

  // Initialize LPeg lexer.
  QColor base = palette().color(QPalette::Base);
  QColor text = palette().color(QPalette::Text);
  bool dark = (text.lightnessF() > base.lightnessF());

  setLexerLanguage("lpeg");
  setProperty("lexer.lpeg.home", Settings::lexerDir().path());
  setProperty("lexer.lpeg.themes", theme->dir().path());
  setProperty("lexer.lpeg.theme", theme->name());
  setProperty("lexer.lpeg.theme.mode", dark ? "dark" : "light");

moving all related code to the end of the constructor if this is working in own function which will be called also for those events

@Murmele The changeEvent is not called on the TextEditors so i have added a static list that holds all instances in order to refresh the theme, but if i call all that theme related stuff inside its own function only the text color is changed, the background and highlight colors stay the same. I don't know Scintilla at all so i have no idea how to implement this atm.

[Murmele]

I will have a look next days

[Murmele]

@0verEngineer can you enable that I have write permission to your branch? Otherwise I cannot push changes (If I remember me correctly you can do it here in this PR on the right side somewhere)

[0verEngineer]

@Murmele It is already enabled and i have nothing changed after creating the fork

[Murmele]

ah sorry was on the _test branch and not on the one from this PR. I am not able to get the changeEvent triggered with windows 10, I am trying later on linux.

[0verEngineer]

ah sorry was on the _test branch and not on the one from this PR. I am not able to get the changeEvent triggered with windows 10, I am trying later on linux.

Ahh this is my fault, i did the testing if the events are triggered with qt 6.2...

It is not triggered on KDE either.

[Murmele]

From here:

Currently there seems to be no way to conect to a signal or event that shows the theme has changed in Windows. So I connected to a signal from a QTimer that fires every 5 seconds to check windowsIsInDarkTheme().

Instead of this workaround I would let it be for all systems which do not support it and once we are going with Qt6 it is working automatically. @0verEngineer can you try my changes? Still we have to find a way to get rid of this TextEdit list. I don't like it ^^

[0verEngineer]

@Murmele I also don't like the TextEdit list :D

I have just tested your changes but it does not change anything about the TextEditor and i also found 2 problems in the normal theming

[Murmele]

@Murmele I also don't like the TextEdit list :D

I have just tested your changes but it does not change anything about the TextEditor and i also found 2 problems in the normal theming

@0verEngineer Sorry for comming back that late. This problem does not exist on Windows and I never realized it on Linux so I assume it is also not there. The problem is, that I was currently not able to get the switching reproducable. I have to check how to get the switching work

[0verEngineer]

@Murmele Is MainWindow::changeEvent also not called on Windows? If not then we should just delay this stuff until Gittyup is migrated to qt6 where the changeEvent works.

[Murmele]

No I did not see. I think if you get it work completely for macos this is fine for me and we can merge it. Once we found a solution on windows / linux we are changing it or it might be resolved automatically with qt6.

[LenaWil]

No I did not see. I think if you get it work completely for macos this is fine for me and we can merge it. Once we found a solution on windows / linux we are changing it or it might be resolved automatically with qt6.

Yeah, I agree, I commented because I thought it might be really easy to also implement it, but if that’s not the case, it might be best to focus on macOS.

[0verEngineer]

I will try to fix the missing stuff for the branch selector and the scintilla stuff this or next weekend, but i have no idea how scintilla works.

Is there an active contributor who has scintilla knowledge that i can contact if i'm stuck? @Murmele

[Murmele]

Hi @0verEngineer I don't have that much experience in it. Next two weeks I am quite busy, but then maybe I can give a try

[Murmele]

I will check if we can switch to Qt6 and hopefully then this is working then as well

[LenaWil]

Any updates?

[Murmele]

Any updates?

I am working on a Qt6 build now. Once it is finished, this can be rebased on it. Then we can check it again

[Murmele]

Is there anybody which can fix the build error in my Qt6 branch? https://github.com/Murmele/Gittyup/pull/429

[Murmele]

Can you try to rebase this one on top of the Qt6 branch to see if it works?