Learn how easy it is to sync an existing GitHub or Google Code repo to a SourceForge project! See Demo

Close

#64 "Require Source Selection" doesn't work with {Fu}

closed-fixed
Emil Brink
Commands (9)
7
2011-10-04
2011-04-11
Skotlex
No

If a command uses both {Fu} in its definition and "require source/destination selection", it appears that the selection is actually cleared BEFORE doing the "require selection" check, thus making the command to not trigger at all.

If I use {fu} then it correctly works.

Without "require selection" Gentoo will launch the command N times, with the first execution receiving all the files, and the following ones starting up without any arguments :/

Discussion

  • Skotlex
    Skotlex
    2011-04-11

    Okay, reading a bit around the code, I've found the cause of the problem.

    The "require selection" flag is checked in csq_handle_ba_flags() (cmdseq.c), and it is normally invoked in fork_and_execute(). However, unselecting items is done in cpr_parse(), which is done BEFORE this check. You can see the logic in the function execute_external().

    Solution? You can change the logic around, move "csq_handle_ba_flags" from fork_and_execute to execute_external.

    HOWEVER, there is a related bug. "Rescan source/destination panes" is triggered from csq_handle_ba_flags also, and these two options are triggering a refresh of the pane contents BEFORE actually launching the external application!

    Obviously, csq_handle_ba_flags needs to be split into two, as two sets of flags need to be checked before running the command, and the other two need to be run afterwards.

    I add a patch which I believe should be a sensible solution to this problem. My only "doubt" with it is the replacement of the function csq_handle_ba_flags to csq_handle_after_flags in idle_handler() (queue.c). Since the call for the function there doesn't check for the return value, I am guessing its only purpose is to run the post-command options.

     
  • Skotlex
    Skotlex
    2011-04-11

    Proposed fix to this bug.

     
    Attachments
  • Skotlex
    Skotlex
    2011-08-28

    Any update on this bug? I mean, it shows some real functional problems with some options in Gentoo, and even supplies a patch that solves them, yet there is no word on what kind of resolution will be applied.

    And it was submitted five months ago. It gets a little tiring to handle my own overlay to install Gentoo in my system. :S

     
  • Emil Brink
    Emil Brink
    2011-08-29

    • priority: 5 --> 3
     
  • Emil Brink
    Emil Brink
    2011-08-29

    Sorry that I didn't get around to addressing this issue in the latest release. It's certainly not forgotten, and I agree that it indicates a real problem.

    I haven't evaluated the patch, but I will of course do so in the course of fixing this.

    I'm definitely aiming to fix it before the next release. I've upgraded the priority a couple of notches, too.

    Thanks for your help.

     
  • Emil Brink
    Emil Brink
    2011-08-29

    Fixed priority, was accidentally decreased rather than increased. Oops.

     
  • Emil Brink
    Emil Brink
    2011-08-29

    • priority: 3 --> 7
     
  • Emil Brink
    Emil Brink
    2011-09-24

    I'm pretty certain I've fixed this, by applying the same solution outlined in Skotlex' patch. I don't really agree with the conclusions in the original comment below about splitting the function in two (for before vs after), since the sets are actually stored distinctly, and separated in the UI.

     
  • Emil Brink
    Emil Brink
    2011-09-24

    • status: open --> open-fixed
     
  • Emil Brink
    Emil Brink
    2011-10-04

    Closing this, fix is in 0.19.12 which was just released. Thanks for reporting.

     
  • Emil Brink
    Emil Brink
    2011-10-04

    • status: open-fixed --> closed-fixed