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

[Taskbar Feature Request] Add a {name} field #982

Closed
qoheniac opened this issue Jan 15, 2021 · 22 comments · Fixed by #1375
Closed

[Taskbar Feature Request] Add a {name} field #982

qoheniac opened this issue Jan 15, 2021 · 22 comments · Fixed by #1375

Comments

@qoheniac
Copy link

A {name} field that works like this: if a .desktop file is found it returns the value of Name and app_id else.

@savchenko
Copy link

Sounds like it can be covered as the part of #1000

@WillPower3309
Copy link

similar functionality already exists. Try title. Its not the .desktop filename but better than app_id if you'd like a solution that works now

    "wlr/taskbar": {
        "format": "{icon} {title}",
        "icon-size": 13,
        "tooltip": false,
        "on-click": "activate",
        "on-click-right": "close"
    },

@qoheniac
Copy link
Author

Thank you, but I knew about {title} and I prefer {app_id}. On the one hand {title} is often very long and for some programs on the other hand it's empty.

@savchenko
Copy link

@WillPower3309 , neat tweak, thanks for sharing!

@qoheniac , maybe having max-length defined on per-module basis would help? It seems like wlr/taskbar already does 90% of it.

@qoheniac
Copy link
Author

(A maybe capitalized version of) {app_id} (or a shortened version of {title}) would be OK for many applications but for many others this produces misleading and/or ugly labels. The expected label would be the value of Name from the .desktop file as that's what launchers use and I think this should be possible as wlr/taskbar is already looking for the desktop files searching for icons. So if it found a .desktop file it could also read the Name field and provide that through {name}.

@qoheniac
Copy link
Author

Here is what I mean:

app_info = Gio::DesktopAppInfo::create_from_filename(prefix + folder + app_id + suffix);

This app_info should already contain the value of Name if I understand it correctly.

@qoheniac
Copy link
Author

qoheniac commented Feb 17, 2021

app_info->get_display_name() should return what I would like to be provided through a {name} placeholder.

Edit: (A capitalized version of) app_id could be used as a fallback if no .desktop file was found or it contained no Name field.

@WillPower3309
Copy link

If this isn't implemented by end of April I can look into it then (exams will be done then)

@WillPower3309
Copy link

WillPower3309 commented Mar 5, 2021

Did some investigation because needed a study break:p
Here is a snippet I changed in src/modules/wlr/taskbar.cpp just to see if I could get proper name printing to console. I basically just added some lines to the handle_app_id function to get it to print when set to app_id in config. Seems like most of the applications on my system didn't work with app_info. I had to add a check to see if it exists or else I'd segfault. That being said, those that did printed their names to the console perfectly. If anyone knows why this only works on some applications, or can provide a function call that works with any application I could get this working. Otherwise, we may have to have a number of fallbacks like how the icons are dealt with, with the final fallback just being app_id, as mentioned above. Also I'm primarily a C dev so apologies for any tough to look at lines :p most of it was copied from the icon implementation.

void Task::handle_app_id(const char *app_id)
{

    static std::vector<std::string> prefixes = search_prefix();

    std::vector<std::string> app_folders = {
        "",
        "applications/",
        "applications/kde/",
        "applications/org.kde."
    };

    std::vector<std::string> suffixes = {
        "",
        ".desktop"
    };

    Glib::RefPtr<Gio::DesktopAppInfo> app_info;

    for (auto& prefix : prefixes)
        for (auto& folder : app_folders)
            for (auto& suffix : suffixes)
                if (!app_info)
                    app_info = Gio::DesktopAppInfo::create_from_filename(prefix + folder + app_id + suffix);

    if(app_info) {
        printf("APP NAME: %s\n", app_info->get_display_name().c_str());
    }
    app_id_ = app_id;

    if (!with_icon_)
        return;

    int icon_size = config_["icon-size"].isInt() ? config_["icon-size"].asInt() : 16;
    bool found = false;
    for (auto& icon_theme : tbar_->icon_themes()) {
        if (image_load_icon(icon_, icon_theme, app_id_, icon_size)) {
            found = true;
            break;
        }
    }

    if (found)
        icon_.show();
    else
        spdlog::debug("Couldn't find icon for {}", app_id_);

}

@WillPower3309
Copy link

WillPower3309 commented Mar 5, 2021

The above is very much a rough test built onto handle_app_id, obviously a much more polished version would be made for any potential pull requests as handle_app_name

@WillPower3309
Copy link

WillPower3309 commented Mar 9, 2021

Sad update: due to the heavy integration of wlr-foreign-toplevel-management-unstable-v1-client-protocol, I'm having isses with the name, as it appears to only support title and app_id, so I can't even add a callback. Unfortunately, this is on the WLR side of things. I'm unsure how to call my implementation of Task::handle_name(const char *app_id), but can verify that it works when it is used in place of handle_app_id.

My changes are on my branch: https://github.com/WillPower3309/waybar

TLDR: I don't know how to have waybar call my handle_name function when name is set in the waybar config. If anyone can point me in the right direction I can finalize this.

@savchenko
Copy link

@Alexays , could you please provide an advice here?

@WillPower3309
Copy link

@Alexays , could you please provide an advice here?

Alternatively, would a pull request be accepted that changes the app_id behaviour to default to name, and display app_id if name doesn't exist? I can't think of a use case where someone would prefer app_id over the application name

@Alexays
Copy link
Owner

Alexays commented Apr 12, 2021

I would rather go for another field like name @WillPower3309

@Anakael
Copy link
Contributor

Anakael commented Aug 10, 2021

I think I can implement it.

@WillPower3309
Copy link

I also put my work on it up, I finished it before I realized that @Anakael did a sweet job of getting it done! Posted my PR anyway in case if it's preferred

@qoheniac
Copy link
Author

qoheniac commented Jan 4, 2022

I just built the master branch to see how #1221 works and the name field looks really good. I just noted one thing, if I run waybar, quit it and repeat that process, it sometimes finds all tasks, sometimes some and sometimes none at all. I'm not sure though if this is related to @Anakael's code or some other changes between current master and the latest release version.

@Anakael
Copy link
Contributor

Anakael commented Jan 4, 2022

I just built the master branch to see how #1221 works and the name field looks really good. I just noted one thing, if I run waybar, quit it and repeat that process, it sometimes finds all tasks, sometimes some and sometimes none at all. I'm not sure though if this is related to @Anakael's code or some other changes between current master and the latest release version.

I'll investigate this. It didn't happen before, did it?

@qoheniac
Copy link
Author

qoheniac commented Jan 4, 2022

It doesn't happen with Waybar v0.9.8 but with my built of the current state of the master branch. I did not check any versions in between.

@qoheniac
Copy link
Author

qoheniac commented Jan 4, 2022

I just built the version right before your changes got merged, i. e.

git checkout 13d25d403e3823fed06ec76f629c1dc47e4d6b51

and the version right after your changes, so

git checkout 8ec321ddaf9f763aa4f07ca1852722fedcf884bb

and that's where the behavior emerges. So the problem is somewhere in #1221.

@Anakael
Copy link
Contributor

Anakael commented Jan 4, 2022

I just built the version right before your changes got merged, i. e.

git checkout 13d25d403e3823fed06ec76f629c1dc47e4d6b51

and the version right after your changes, so

git checkout 8ec321ddaf9f763aa4f07ca1852722fedcf884bb

and that's where the behavior emerges. So the problem is somewhere in #1221.

Ok. I'll check it today.

@qoheniac
Copy link
Author

qoheniac commented Jan 5, 2022

Perfect, love it!

qoheniac added a commit to qoheniac/config that referenced this issue Jan 18, 2022
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 a pull request may close this issue.

5 participants