• Markaos@discuss.tchncs.de
    link
    fedilink
    arrow-up
    2
    ·
    18 hours ago

    Yes, the DE-specific implementations is pointless (as far as I know, I use a WM), but the XDG implementation is actually used first, and the function returns true if any impl returns true, like xdg() || gnome() || gnome_old() || kde().

    True, I must’ve read the code wrong when making the comment.

    This isn’t that bad?

    Yes, which is why I take issue with a PR (or rather what should have been a PR) that introduces crap code with clearly visible low effort improvements - the submitter should’ve already done that so the project doesn’t unnecessarily gain technical debt by accepting the change.

    With multiple impls, you have to resolve conflicts somehow.

    Yep, that’s why I think it’s important for the implementations to actually differentiate between light and fail state - that’s the smallest change and allows you to keep the whole detection logic in the individual implementations. Combine that with XDG being the default/first one and you get something reasonable (in a world where the separate implementations are necessary). You do mention this, but I feel like the whole two paragraphs are just expanding on this idea.

    But it’s better to criticize the code’s actual faults (…)

    I made a mistake with the order in which the implementations are called, but I consider the rest of the comment to still stand and the criticisms to be valid.