Last action executed when escaping from command entry

[Chad Parker]

Steps to reproduce:

1. Configure the command entry to be in the status bar
2. Open the log window
3. Press ":"
4. Enter some characters, e.g. "asdfdhf"
5. Press enter -> "no action asdfdhf()" appears in log window
6. Press ":"
7. Press esc -> "no action asdfdhf()" appears in log window a second time

This is happening because of an else if clause in ghid_handle_user_command:src/hid/gtk/gui-command-window.c. If ghid_command_entry_get returns NULL, then the first if fails, but the second if will succeed if there's anything in the history list, and execute the previous action.

This was clearly done deliberately, but I have no idea why. It seems kind of dangerous to me to execute a previous action if you fail to get a new one.

[Chad Parker (parker-charles)]

Branch LP1674133 removes the code that causes this.

[Bert Timmerman (bert-timmerman)]

Hi Charles,

I tested your patch, and we're not finished with this bug yet ;-)

In a slightly different testing scenario a segfault occurs.

Steps to reproduce:

1. Configure the command entry to be NOT in the status bar
2. Open the log window
3. Press ":"
4. Enter some characters, e.g. "asdfdhf" (do not execute this command by
   pressing [enter]
5. Mouse click [x] to kill the command entry dialog
6. Press ":"
7. Press esc -> segmentation fault (core dumped)

Yet another testing scenario gives the following violation against the
principle of "least surprise":

1. Configure the command entry to be NOT in the status bar
2. Open the log window
3. Press ":"
4. Enter some characters, e.g. "asdfdhf" (do not execute this command by
   pressing [enter])
5. Press esc to escape from the command entry dialog and to close it ->
   nothing happens

versus:

1. Configure the command entry to be in the status bar
2. Open the log window
3. Press ":"
4. Enter some characters, e.g. "asdfdhf" (do not execute this command by
   pressing [enter])
5. Press esc to escape from the command entry and close it -> user
   experience (UX) as expected

Could you please confirm if these new scenarios happen in the same manner for you ?

PCB needs some more in depth analyses of involved code paths and TLC to
get its behaviour consistent ;-)

On a side note: AFAICT this snippet is in the code base as of 2005/2006
and Dan and DJ last touched this, they may remember why it was put there
in the first place.

With kind regards,

Bert Timmerman.

[Chad Parker (parker-charles)]

Hi Bert-

Thanks for taking a look.

1. Configure the command entry to be NOT in the status bar
> 2. Open the log window
> 3. Press ":"
> 4. Enter some characters, e.g. "asdfdhf" (do not execute this command by
> pressing [enter]
> 5. Mouse click [x] to kill the command entry dialog
> 6. Press ":"
> 7. Press esc -> segmentation fault (core dumped)
>

I thought that I had tested this! But I do get the segfault... sorry. I'll
look into it.

>
> Yet another testing scenario gives the following violation against the
> principle of "least surprise":
>
> 1. Configure the command entry to be NOT in the status bar
> 2. Open the log window
> 3. Press ":"
> 4. Enter some characters, e.g. "asdfdhf" (do not execute this command by
> pressing [enter])
> 5. Press esc to escape from the command entry dialog and to close it ->
> nothing happens
>
> versus:
>
> 1. Configure the command entry to be in the status bar
> 2. Open the log window
> 3. Press ":"
> 4. Enter some characters, e.g. "asdfdhf" (do not execute this command by
> pressing [enter])
> 5. Press esc to escape from the command entry and close it -> user
> experience (UX) as expected
>
> Could you please confirm if these new scenarios happen in the same
> manner for you ?
>

So, basically you just want to close the command entry window with esc,
like when it's in the status bar? I'll point out that that isn't the
behavior of 4.0.0, so, I didn't break this, but I it should be doable. I'll
take a look here too.

Thanks,
--Chad

[Chad Parker (parker-charles)]

Hi Bert-

I looked quickly at the segfault. I think that's the issue that I fixed in
the other related patch: LP1672886. When I rebase this branch against
master, the segfault no longer happens. I pushed the rebased branch (had to
force it).

Maybe we can open another bug report for the user experience enhancement?

Thanks,
--Chad

On Sun, Apr 30, 2017 at 5:21 PM, Chad Parker <email address hidden>
wrote:

> Hi Bert-
>
> Thanks for taking a look.
>
> 1. Configure the command entry to be NOT in the status bar
>> 2. Open the log window
>> 3. Press ":"
>> 4. Enter some characters, e.g. "asdfdhf" (do not execute this command by
>> pressing [enter]
>> 5. Mouse click [x] to kill the command entry dialog
>> 6. Press ":"
>> 7. Press esc -> segmentation fault (core dumped)
>>
>
> I thought that I had tested this! But I do get the segfault... sorry. I'll
> look into it.
>
>
>>
>> Yet another testing scenario gives the following violation against the
>> principle of "least surprise":
>>
>> 1. Configure the command entry to be NOT in the status bar
>> 2. Open the log window
>> 3. Press ":"
>> 4. Enter some characters, e.g. "asdfdhf" (do not execute this command by
>> pressing [enter])
>> 5. Press esc to escape from the command entry dialog and to close it ->
>> nothing happens
>>
>> versus:
>>
>> 1. Configure the command entry to be in the status bar
>> 2. Open the log window
>> 3. Press ":"
>> 4. Enter some characters, e.g. "asdfdhf" (do not execute this command by
>> pressing [enter])
>> 5. Press esc to escape from the command entry and close it -> user
>> experience (UX) as expected
>>
>> Could you please confirm if these new scenarios happen in the same
>> manner for you ?
>>
>
> So, basically you just want to close the command entry window with esc,
> like when it's in the status bar? I'll point out that that isn't the
> behavior of 4.0.0, so, I didn't break this, but I it should be doable. I'll
> take a look here too.
>
> Thanks,
> --Chad
>
>

[Bert Timmerman (bert-timmerman)]

Hi Charles,

Recently Igor2 solved this issue in pcb-rnd.

While we were discussing this in #geda he agreed on your patch (I take that as a positive review) and also pointed me to the attached diff from pcb-rnd.

I hope this helps.

Kind regards,

Bert Timmerman.

[Bert Timmerman (bert-timmerman)]

Hi Charles,

I cherry-picked this single commit to master.

Thanks and with kind regards,

Bert Timmerman.