Skip to content

Conversation

azubieta
Copy link
Collaborator

@azubieta azubieta commented Apr 6, 2021

This is an attempt to include the application information in the "Integration Dialog". It uses the information contained in the Desktop file.

Related to #150

Screenshot_20210412_221958

@azubieta azubieta force-pushed the feature/improve_integration_dialog branch from 605c805 to 35b2d95 Compare April 13, 2021 03:14
@azubieta azubieta changed the title Show target application Icon, Name, Description and Categories in the integration dialog Show target application Icon, Name, and Description in the integration dialog Apr 13, 2021
@azubieta azubieta force-pushed the feature/improve_integration_dialog branch from 35b2d95 to 9580cec Compare April 13, 2021 03:25
message = message.arg(integratedAppImagesDestinationPath);
ui->message->setText(message);
} catch (appimage::core::AppImageError& error) {
// TODO: Properly handle errors
Copy link
Collaborator Author

@azubieta azubieta Apr 13, 2021

Choose a reason for hiding this comment

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

Ideas are welcome here!

@azubieta azubieta marked this pull request as ready for review April 13, 2021 03:27
@azubieta azubieta requested a review from TheAssassin April 13, 2021 03:27
@TheAssassin
Copy link
Owner

I think this feature must be made optional for now. Adding third-party dependencies like this one has an impact on packaging in distros, and your desktop utils parser is not available anywhere AFAIK.

# main AppImageLauncher application
add_executable(AppImageLauncher main.cpp resources.qrc first-run.cpp first-run.h first-run.ui integration_dialog.cpp integration_dialog.h integration_dialog.ui)
target_link_libraries(AppImageLauncher shared PkgConfig::glib libappimage shared)
target_link_libraries(AppImageLauncher shared PkgConfig::glib libappimage shared XdgUtils::DesktopEntry)
Copy link
Owner

Choose a reason for hiding this comment

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

Where is this dependency loaded? I presume you forgot to commit a line in some other file...?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It comes as a dependency of libappimage

@azubieta
Copy link
Collaborator Author

I think this feature must be made optional for now. Adding third-party dependencies like this one has an impact on packaging in distros, and your desktop utils parser is not available anywhere AFAIK.

This is a dependency of libappimage so if a distro is shipping the latest libappimage is also shipping this one.

@TheAssassin
Copy link
Owner

I'm sorry your PR has been open for all these weeks. I am not sure I'll get to reviewing it any soon. Please don't be disappointed. I'm busy with other things than my open-source work at the moment. Things will relax in a couple of months.

@gamedevsam
Copy link

This seems like a legitimate improvement to the UI. Poke @TheAssassin.

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.

3 participants