Ticket #3699 (new enhancement)

Opened 7 years ago

Last modified 7 years ago

patch to fix restoring of the directory contents of the active panel on startup when "Auto Save Panel Setup" option is enabled.

Reported by: Miven Owned by:
Priority: major Milestone: Future Releases
Component: mc-core Version: master
Keywords: autosave panels startup active panel Cc:
Blocked By: Blocking:
Branch state: no branch Votes for changeset:

Description

This is a patch for {setup.[ch], main.c, midnight.c} to fix the restoring of the directory contents of the active panel on startup when "Auto Save Panel Setup" option is enabled, as per a user request on the mailing list mc@….

Change History

comment:1 follow-up: ↓ 4 Changed 7 years ago by andrew_b

Please don't attach compressed files, especially patches. They cannot be viewed on-line here.

The patch is incomplete: new behavior is not documented.

The patch looks like dirty hack.

comment:2 follow-up: ↓ 3 Changed 7 years ago by zaytsev

Could you please specify what's particularly dirty about this hack, other than the lack of documentation and tests? :-) As far as I can see, the new code is simply doing the same what is already done for the other panel.

comment:3 in reply to: ↑ 2 Changed 7 years ago by andrew_b

Replying to zaytsev:

Could you please specify what's particularly dirty about this hack

The hack is use of saving option during loading. When you're loading options, why you should take care about how that options were saved? When they were saved? If you have saved_this_dir in config, you just must use it.
You should take care about value of saved_this_dir during saving panel options: make it empty or contained panel cwd.

comment:4 in reply to: ↑ 1 Changed 7 years ago by Miven

Replying to andrew_b:

Please don't attach compressed files, especially patches. They cannot be viewed on-line here.

Yes, thank you for fixing that. Trac didn't say what type of file it expected. I don't submit many patches these days, so I'm unfamiliar with the process. Often gzip files are automatically un-zipped by the handling software.

The patch is incomplete: new behavior is not documented.

The patch looks like dirty hack.

I agree, it is. I posted it to assist a user on the mailing list who had the need for it, and then another user reminded me it will get lost if I don't put it on the Trac, so here it is.

I personally don't use the "auto-save panels setup" feature, but I used to years ago with 4.6 series, and I figure if it's a feature, it should do what it says. The 'dirty hack' is so simple there's not much testing to do. It either does what it does, or it doesn't. There's only 2 variables to handle (if A then B). It did what I wanted it to do (save and restore the active panel contents under certain conditions), so I couldn't figure out how to test it beyond that.

Thank you for having a look at the patch. I believe "auto-save panels setup" has been overlooked in the code re-work from 4.6 to 4.8. Obviously someone needed it in the past, because it's in there, so we might as well make sure it does what it says. Really, it needs a sub-option "Restore active panel", which is what this patch *actually* does.

Note: See TracTickets for help on using tickets.