-
Notifications
You must be signed in to change notification settings - Fork 33
fix icon import #198
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
base: master
Are you sure you want to change the base?
fix icon import #198
Conversation
How did you write that code if you're not a C++ dev, though? |
AI and linux knowledge lol |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Had a first glance. This is quite a lot of code and it is not clear which part solves the problem, really.
std::string baseName = desktopEntryIconName; | ||
size_t lastDot = baseName.find_last_of('.'); | ||
if (lastDot != std::string::npos) { | ||
baseName = baseName.substr(lastDot + 1); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This bit is duplicated at least three times. It should be a utility function.
for (const auto& path : hicolorIconPaths) { | ||
// Extract size from path like "usr/share/icons/hicolor/512x512/apps/" | ||
std::string sizeStr; | ||
size_t sizePos = path.find("/hicolor/"); | ||
if (sizePos != std::string::npos) { | ||
sizePos = path.find('/', sizePos + 9); | ||
if (sizePos != std::string::npos) { | ||
size_t endPos = path.find('/', sizePos + 1); | ||
if (endPos != std::string::npos) { | ||
sizeStr = path.substr(sizePos + 1, endPos - sizePos - 1); | ||
// Handle NxN format | ||
size_t xPos = sizeStr.find('x'); | ||
if (xPos != std::string::npos) { | ||
try { | ||
int size = std::stoi(sizeStr.substr(0, xPos)); | ||
if (size > largestSize) { | ||
largestSize = size; | ||
largestIconPath = path; | ||
} | ||
} catch (...) { | ||
// Skip invalid size format | ||
continue; | ||
} | ||
} | ||
} | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This block of code is really hard to read and understand and does not actually validate the path structure. But given the amount of nested functionality, this also should be a separate function.
I am not a c++ dev - this was literally done because my cursor AppImage had no icon.
i know its not the best solution. probably going to have issues with some appimages packaging their icons in weird places