Internal: make arguments in TTabBar more contextualised#4803
Internal: make arguments in TTabBar more contextualised#4803vadi2 wants to merge 1 commit intoMudlet:developmentfrom
Conversation
|
Hey there! Thanks for helping Mudlet improve. 🌟 Test versionsYou can directly test the changes here:
No need to install anything - just unzip and run. |
| } | ||
|
|
||
| void TStyle::setNamedTabState(const QString& text, const bool state, QSet<QString>& effect) | ||
| void TStyle::setNamedTabState(const QString& profileName, const bool state, QSet<QString>& effect) |
There was a problem hiding this comment.
That wasn't quite what I meant - notice how the methods that have "Name" in their name (like this:
"set Name dTabState"
perhaps it would be better if text as a parameter/argument was replaced by name within this class - which would be a little more consistent here and go some way towards your wording:
| void TStyle::setNamedTabState(const QString& profileName, const bool state, QSet<QString>& effect) | |
| void TStyle::setNamedTabState(const QString& name, const bool state, QSet<QString>& effect) |
There was a problem hiding this comment.
I don't follow - because the named tab's name is the profile name.
There was a problem hiding this comment.
Then again, there is a separation of concerns...
There was a problem hiding this comment.
I don't follow - because the named tab's name is the profile name.
Then again, there is a separation of concerns...
In this usage - sure - but if we (or some other project) used the class somewhere else, then profileName doesn't fit.
OTOH Since the methods refer to the text on the tab as the "text" - in setNamedTabState and tabName and as the argument to the methods that take a QString argument to select a particular tab as opposed to those that take an ìnteger and refer to that number as the "index" - it seems reasonable to me {and I would accept without much grumbling} a change from text to name - profileName just doesn't sit well with me. 🙁
💡 Say, for instance, if we redid the Editor to use this class to put the different Mudlet items {"aliases", "buttons", "keybindings" etc.} as tabs across the top - instead of the current buttons on the side; then "name" would still fit - but "profileName" wouldn't.
🤞 hoping to avoid: 🚲 🚲 🏠 🚲 🚲
Brief overview of PR changes/additions
Internal: make arguments in TTabBar more contextualised
Motivation for adding to Mudlet
Easier dev time
Other info (issues closed, discussion etc)
#4788 (comment)