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

fix: work on aquamarine backend #54

Merged
merged 3 commits into from
Jul 22, 2024
Merged

fix: work on aquamarine backend #54

merged 3 commits into from
Jul 22, 2024

Conversation

WhySoBad
Copy link
Collaborator

The new aquamarine backend no longer uses an char pointer array but the string from the standard library which cannot be a nullptr

@@ -184,5 +184,5 @@ Layout VirtualDesk::generateCurrentMonitorLayout() {
std::string VirtualDesk::monitorDesc(const CMonitor* monitor) {
if (!monitor->output)
return monitor->szName;
return monitor->output->description ? monitor->output->description : monitor->szName;
return monitor->output->description;
Copy link
Owner

Choose a reason for hiding this comment

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

I think this should actually be:

Suggested change
return monitor->output->description;
return monitor->output->description.empty() ? monitor->szName : monitor->output->description;

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes you're right. I thought the description is always set.

Additionally, I've found the monitor->szDescription field exists. This should be the same as the monitor->output->description but at the convenience we don't have to go over the output which is more likely to have breaking changes

@levnikmyskin
Copy link
Owner

Ah also, I see you're merging directly into main, could you merge into dev instead? Thanks 🙏

@WhySoBad WhySoBad changed the base branch from main to dev July 22, 2024 11:57
@WhySoBad
Copy link
Collaborator Author

Ah also, I see you're merging directly into main, could you merge into dev instead? Thanks 🙏

Thanks for the reminder, I totally forgot to change the target branch

Somehow, yesterday the build succeeded without installing aquamarine. I've now added the aquamarine dependency to the CI as well as the libxcursor package as the CI was failing without these packages today

Copy link
Owner

@levnikmyskin levnikmyskin left a comment

Choose a reason for hiding this comment

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

LGTM now! Also, once again, thank you so much for your effort in keeping this plugin up to date, it is very much appreciated :)

@levnikmyskin levnikmyskin merged commit ee9eb63 into dev Jul 22, 2024
1 check passed
@levnikmyskin levnikmyskin deleted the move-to-aquamarine branch July 22, 2024 13:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants