bernds / q5Go

A tool for Go players: SGF editor, analysis tool, game database and pattern search tool, IGS client
GNU General Public License v2.0
171 stars 28 forks source link

Server command input line does not respond. #33

Closed colingeniet closed 4 years ago

colingeniet commented 4 years ago

Steps to reproduce:

The issue does not occurs before opening the new board window. Closing that second window does not solve it. An actual server connection is not necessary, this seems to be purely on the UI side.

This is on the master branch, running: Linux 5.7.3 X11 i3 4.18.1 Qt 5.14.2

bernds commented 4 years ago

I can't seem to reproduce this specific behaviour, but I know of one weird issue where you can't send the same command twice. Is that what you are seeing?

colingeniet commented 4 years ago

I know of one weird issue where you can't send the same command twice. Is that what you are seeing?

Yes, looks like I completely misinterpreted the issue, forget the opening a new window part.

It looks like it is somehow related to the command history. Maybe something to do with autocompletion shortcuts?

bernds commented 4 years ago

I suspect it's to do with the entry field being a combo box and maybe that's disallowing duplicates. It's something that never quite rose to the top of the to-do list. Maybe now that someone else has noticed...

colingeniet commented 4 years ago

After a quick glance at the code, the behaviour of ClientWindow::slot_cmdactivated_int seems strange to me.

void ClientWindow::slot_cmdactivated_int(int)
{
    if (cb_cmdLine->count() > cmd_count)
    {
        cmd_count = cb_cmdLine->count();
        cmd_valid = true;
    }
    else
        cmd_valid = false;
}

This callback is the only place where cmd_count is modified. Thus the condition tests if the ComboBox list has grown since the callback was last called.

If the command entered was previously in the list, then the list does not grow when sending the command, hence it the condition fails and it is marked as invalid.

I can confirm that this is indeed what happens. Adding the following debug line:

void ClientWindow::slot_cmdactivated_int(selected)
{
    qDebug("%i %i", cmd_count, cb_cmdLine->count());

gives the following log (comments with # added)

# send command 'test' for the first time
0 2
cmd_valid: 1
CMD:  "test"
"Command skipped - no telnet connection: test"
# send command 'test' a second time
2 2
cmd_valid: 0
# send command 'test2' for the first time
2 3
cmd_valid: 1
CMD:  "test2"
"Command skipped - no telnet connection: test2"

When repeating the command, the size of the list remains 2, and the test 2 < 2 fails.

EDIT: Disregard this, with context from your previous comment, I understand how it is designed with having duplicates enabled in mind.

colingeniet commented 4 years ago

That's disallowing duplicates

How about: https://doc.qt.io/qt-5/qcombobox.html#duplicatesEnabled-prop

bernds commented 4 years ago

In an ideal world, you'd want to allow entering them in the text box, but not recording them each time. But setting that property is indeed an improvement.