Ticket #2967 (reopened defect)

Opened 6 years ago

Last modified 3 years ago

"Screen list" and related M-`, M-{ and M-} do not work from "find file" list

Reported by: szaszg Owned by:
Priority: major Milestone: Future Releases
Component: mc-core Version: master
Keywords: Cc: gotar@…
Blocked By: Blocking:
Branch state: no branch Votes for changeset:

Description

From panels: Find File -> open a file from the list and now M-`, M-{ and M-} hotkeys do not work.

Attachments

mc.modal.diff (3.1 KB) - added by szaszg 5 years ago.

Change History

comment:1 Changed 6 years ago by andrew_b

  • Status changed from new to closed
  • Resolution set to wontfix
  • Milestone Future Releases deleted

Editor/viewer opened from find result is modal and doesn't provide switch to another screen. This is a feature.

comment:2 Changed 6 years ago by szaszg

Hmm.. This is not a feature, this is a very annoying behavior.

comment:3 Changed 6 years ago by gotar

  • Cc gotar@… added

Changed 5 years ago by szaszg

comment:4 follow-up: ↓ 5 Changed 5 years ago by szaszg

  • Status changed from closed to reopened
  • Resolution wontfix deleted

I was thinking a lot of about this "feature"...

But I did not realize what it's for...

So, i upload a patch to fix this annoying thing.

This patch changes 8 little thing:

  1. lib/widget/dialog.h:

adds a new dlg_flag: DLG_ALLOW_SL -> allow ScreenList (and Prev / Next) for this dialog even if modal

  1. and 3. src/filemanager/find.c:find_parameters ():

uses this new flag for creating dialogs to allow "screen switching" from find file

4., 5. and 6. lib/widget/dialog.c:dlg_execute_cmd ():

allows "screen switching" for dialogs flagged by DLG_ALLOW_SL too

  1. lib/widget/dialog.c:dlg_init ()

remove the code that marks non-modal dialogs as modal

  1. lib/widget/dialog-switch.c:dialog_switch_remove ()

changes the code for select "current dialog" to always select the first dialog ([main]) after user close one...

The last one is a crucial step... because the whole dialog switching code is quite hmm... "hacky"

comment:5 in reply to: ↑ 4 ; follow-up: ↓ 6 Changed 5 years ago by andrew_b

Replying to szaszg:

The last one is a crucial step... because the whole dialog switching code is quite hmm... "hacky"

Yes. And DLG_ALLOW_SL is a hack for hack.

comment:6 in reply to: ↑ 5 Changed 5 years ago by szaszg

Replying to andrew_b:

Yes. And DLG_ALLOW_SL is a hack for hack.

Hmm.. I don't think so... this is just a dialog flag wich allow the key combos to works with the flagged ones... where is the "hack"???

Not the DLG_ALLOW_SL is a hack (or "hack of the hack"), maximum the 8th step: we cannot select a dialog after remove one freely and we always switch back to [main] dialog in dialog_switch_remove(). But this is neccessary because, when editor (and maybe viewer) ends it explicitly update the [main] dialog (even dialog_switch_remove() select an other dialog) with update_panels(), and so [main]->[find file] gain the "focus" not the "selected" one...

So, if somebody rewrite the whole code (of dialog switching), than this 8th step (mc_current = g_list_last(mc_dialog);) may be rewriten too (e.g. to select the previouse dialog, if one closed).
So really the 8th step is not a "hack", just a "neccessity".

(BTW IMHO the hackiest part of the dialog switching code, is the dialog_switch_goto() and everything around them ... :-)

comment:7 Changed 5 years ago by szaszg

BTW.: I am very pleased to talk about this ;-)

May i ask:

  • the modal "flag" why not one of the dialogs' flags? Is there any significance?
  • is there any other intention of the modal "flag", or it just because "ScreenList" things?
  • because MC just a "half-way" event-driven the modal name is a little bit misleading (at the present implementation), isn't it?

Just a word about "DLG_ALLOW_SL is hacky" thing again:

I thinking about it, but i cannot see how we implement this thing cleaner even now or later if the modal and dialog stuff will work similar than other event driven GUI systems... ?
If you have a cleaner implementation plan than "using a flag in the dialog widget to allow ScreenList stuff works/not works", please... :-)

comment:8 Changed 3 years ago by andrew_b

  • Milestone set to Future Releases
Note: See TracTickets for help on using tickets.