Skip to content

Conversation

@vanarp123
Copy link

…ase.

Implements #2585

Checks

Copy link
Member

@Krzmbrzl Krzmbrzl left a comment

Choose a reason for hiding this comment

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

In order to retain backwards compatibility and as a nice gimmick, I think it would be good if we do a case-sensitive search first and if that doesn't find the associated channel, do another case-insensitive search. However, the latter needs to be done to completion (process all channel names) and only if there is only a single match should that be taken. Otherwise, we can emit a warning about ambiguous channel names.

QString str = QString();
while (chan && qlChans.count() > 0) {
QString elem = qlChans.takeFirst().toLower();
QString elem = qlChans.takeFirst(); // Removed .toLower() to preserve case
Copy link
Member

Choose a reason for hiding this comment

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

You don't have to represent the old code in a comment. If someone wants to see what it looked before, they can use git for this purpose.

@Krzmbrzl Krzmbrzl added client bug A bug (error) in the software labels Apr 25, 2025
@davidebeatrici
Copy link
Member

In order to retain backwards compatibility and as a nice gimmick, I think it would be good if we do a case-sensitive search first and if that doesn't find the associated channel, do another case-insensitive search. However, the latter needs to be done to completion (process all channel names) and only if there is only a single match should that be taken. Otherwise, we can emit a warning about ambiguous channel names.

Agreed.

…ase. Now implements secondary case-insensitive search.

Implements mumble-voip#2585
@vanarp123
Copy link
Author

Hello,

I just implemented the suggested changes. Please let me know if you have any other feedback ?

}

// Helper method to find channel with either case-sensitive or case-insensitive search
bool MainWindow::findChannelWithSensitivity(Channel *&chan, QStringList &qlChans, QString &str, bool caseSensitive) {
Copy link
Member

Choose a reason for hiding this comment

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

Delegating the job to a separate function is perfectly fine and appreciated, however I would change the name to something more specific such as:

findChannelWithCasing()

@Krzmbrzl Krzmbrzl marked this pull request as draft June 6, 2025 07:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug A bug (error) in the software client

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants