Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable Recursive Plugin Panels for Plugin.Command Calls #680

Closed
rohitab opened this issue May 7, 2023 · 17 comments
Closed

Enable Recursive Plugin Panels for Plugin.Command Calls #680

rohitab opened this issue May 7, 2023 · 17 comments

Comments

@rohitab
Copy link
Contributor

rohitab commented May 7, 2023

Description of the new feature or improvement

Please refer to commit 7d3c296 which adds an experimental feature for auto plugin panel stack.
See Stack of plug-in panels for details.

This feature works great when launching a plugin from inside another plugin, e.g. PE Analyzer from inside an archive, so thank you! However, if the plugin is launched from a Lua Macro using Plugin.Command, the stack is not saved, and the user is returned back to the file panel instead of the archiver panel.

It would be nice if the auto plugin stack could be enabled for this case as well. I understand that this is an experimental feature and not fully implemented.

Additional Info (if interested in usage): I have a file association for Dex files that runs a Lua macro and converts them to JAR files on pressing Ctrl+PgDn. These Dex files are normally located inside an APK, which is a ZIP archive. After the conversion, the JAR file (which is also a ZIP archive) is opened in the ArcLite plugin using Plugin.Command function. When I exit the archive using Ctrl+PgUp, I get returned to the file panel instead of the ArcLite panel with the APK.

Proposed technical implementation details (optional)

The following code was added to PluginManager::CommandsMenu function to handle the plugin command case in commit 7d3c296

FarManager/far/plugins.cpp

Lines 1599 to 1644 in c085d23

if (hPlugin && !Editor && !Viewer && !Dialog)
{
bool plugin_panel_stack = false;
std::wstring hostfile;
const auto FList = dynamic_cast<FileList*>(ActivePanel.get());
//
if (FList)
{
if (ActivePanel->GetMode() == panel_mode::PLUGIN_PANEL)
{
unsigned long long IFlags = 0;
PluginInfo Info{ sizeof(Info) };
if (GetPluginInfo(hPlugin->plugin(), &Info))
IFlags = Info.Flags;
if ((IFlags & PF_RECURSIVEPANEL) != 0)
{
OpenPanelInfo info = { sizeof(info), hPlugin->panel() };
hPlugin->plugin()->GetOpenPanelInfo(&info);
if (info.HostFile && info.HostFile[0])
{
hostfile = info.HostFile;
plugin_panel_stack = true;
}
}
}
}
if (plugin_panel_stack)
{
FList->PushFilePlugin(hostfile, std::move(hPlugin));
}
else
{
if (ActivePanel->ProcessPluginEvent(FE_CLOSE, nullptr))
{
ClosePanel(std::move(hPlugin));
return FALSE;
}
const auto NewPanel = Global->CtrlObject->Cp()->ChangePanel(ActivePanel, panel_type::FILE_PANEL, TRUE, TRUE);
NewPanel->SetPluginMode(std::move(hPlugin), {}, true);
NewPanel->Update(0);
NewPanel->Show();
}
}

The following code is from the PluginManager::CallPluginItem function. If you replace this code with the above code, the auto plugin stack works with the Plugin.Command function from Lua as well. I just tested it out and it appears to be working.

FarManager/far/plugins.cpp

Lines 2357 to 2371 in c085d23

if (hPlugin && !Editor && !Viewer && !Dialog)
{
//BUGBUG: Закрытие панели? Нужно ли оно?
//BUGBUG: В ProcessCommandLine зовется перед Open, а в CPT_MENU - после
if (ActivePanel->ProcessPluginEvent(FE_CLOSE, nullptr))
{
ClosePanel(std::move(hPlugin));
return false;
}
const auto NewPanel = Global->CtrlObject->Cp()->ChangePanel(ActivePanel, panel_type::FILE_PANEL, TRUE, TRUE);
NewPanel->SetPluginMode(std::move(hPlugin), {}, true);
NewPanel->Update(0);
NewPanel->Show();
}

Of course, the above change requires adding the PF_RECURSIVEPANEL flag to the GetPluginInfoW function in the ArcLite plugin. Adding info->Flags = PF_RECURSIVEPANEL; just below the following line works.

info->StructSize = sizeof(PluginInfo);

To add support for command-line (prefix) calling, the following code in PluginManager::ProcessCommandLine would need to be replaced with the above code, excluding the ProcessPluginEvent part. I tested this change as well and it appears to be working correctly.

FarManager/far/plugins.cpp

Lines 2083 to 2089 in c085d23

if (auto hPlugin = Global->CtrlObject->Plugins->Open(PluginIterator->pPlugin, OPEN_COMMANDLINE, FarUuid, reinterpret_cast<intptr_t>(&info)))
{
const auto NewPanel = Global->CtrlObject->Cp()->ChangePanel(Global->CtrlObject->Cp()->ActivePanel(), panel_type::FILE_PANEL, TRUE, TRUE);
NewPanel->SetPluginMode(std::move(hPlugin), {}, true);
NewPanel->Update(0);
NewPanel->Show();
}

I'm not very familiar with the code so the above changes may have side effects that I'm currently unaware of.

@rohitab
Copy link
Contributor Author

rohitab commented May 7, 2023

@w17 Thanks for adding the auto plugin stack feature. Please take a look at this ticket when you have some time.

@w17
Copy link
Contributor

w17 commented May 11, 2023

@rohitab ok, maybe next weekend...

@w17
Copy link
Contributor

w17 commented May 12, 2023

build 6140

@rohitab
Copy link
Contributor Author

rohitab commented May 13, 2023

Thank you so much! I just tested version 3.0.6141.0 x64 and it works great.

Unfortunately, there appears to be another issue that has arisen due to this. If you have an archive panel open, and you use the command-line prefix arc: to open another archive, the archive is deleted on closing the panel. I will create a separate ticket for this.

I apologize. I should have tested more thoroughly before asking you to make the command-line prefix change.

@w17
Copy link
Contributor

w17 commented May 24, 2023

@rohitab what is your use case for Plugin.Command?

@rohitab
Copy link
Contributor Author

rohitab commented May 25, 2023

@w17, here is my use case.

Use Case

I have a file association for Dex files that runs a Lua macro, which converts them to JAR files on pressing Ctrl+PgDn. These Dex files are normally located inside an APK, which is a ZIP archive. After the conversion, the JAR file (which is also a ZIP archive) is opened in the ArcLite plugin using Plugin.Command function. When I exit the archive using Ctrl+PgUp, I get returned to the file panel instead of the ArcLite panel with the APK.

To simplify, there is an encoded archive within an archive. The encoded archive is decoded to a temporary file and opened using Plugin.Command. On exiting this temporary archive, I would expect to be returned to the outer archive.

Plugin.Command Test Case

A very easy way to test this is by running the following commands, one at a time, using the Far command line input.

cd "%TEMP%"
copy /Y "%FARHOME%\Plugins\FarColorer\base\common.zip" test-one.zip
goto:test-one.zip
copy /Y test-one.zip test-two.zip
lua:Plugin.Command("65642111-AA69-4B84-B4B8-9249579EC4FA", "test-one.zip")
lua:Plugin.Command("65642111-AA69-4B84-B4B8-9249579EC4FA", "test-two.zip")
lua:Keys("CtrlPgUp")

🔴 Results for version 3.0.6133.0 x64: No Recursive Plugin Panels

You will be returned to the file panel with the %TEMP% directory, and the cursor will be on test-two.zip.

🟢 Results for version 3.0.6140.0 x64: Recursive Plugin Panel Support

You will be returned to the archive panel with test-one.zip. Panel title will show ArcLite:zip:test-one.zip
If you press Ctrl+PgUp to exit this archive, you will be returned to the %TEMP% directory, and the cursor will be on test-one.zip.

The only issue here is that the file test-two.zip will get deleted. I reported this issue in #683 where the archive was being deleted when using the plugin prefix arc:. Since I was using temporary files and deleting them myself, I didn't realize that the files were being deleted with Plugin.Command calls as well.

🔴 Results for version 3.0.6153.0 x64: Current Version

You will be returned to the file panel with the %TEMP% directory, and the cursor will be on test-two.zip. Basically, the result is the same as it was before Recursive Plugin Panel support was enabled.

Plugin Prefix Test Case

The following test case can be used for testing plugin prefix arc: with Recursive Plugin Panels. The result is the same as that for Plugin.Command calls.

cd "%TEMP%"
copy /Y "%FARHOME%\Plugins\FarColorer\base\common.zip" test-one.zip
goto:test-one.zip
copy /Y test-one.zip test-two.zip
arc:test-one.zip
arc:test-two.zip
lua:Keys("CtrlPgUp")

@rohitab
Copy link
Contributor Author

rohitab commented May 25, 2023

I just tested version 3.0.6153.0 x64 using -r argument with Plugin.Command, based on the changes you made in commit 3776d9c. It appears to be working fine.

Plugin.Command Test Case (Updated)

The following modified test case, which uses -r for opening test-two.zip, gives the correct results.

cd "%TEMP%"
copy /Y "%FARHOME%\Plugins\FarColorer\base\common.zip" test-one.zip
goto:test-one.zip
copy /Y test-one.zip test-two.zip
lua:Plugin.Command("65642111-AA69-4B84-B4B8-9249579EC4FA", "test-one.zip")
lua:Plugin.Command("65642111-AA69-4B84-B4B8-9249579EC4FA", "-r test-two.zip")
lua:Keys("CtrlPgUp")

🟢 Results for version 3.0.6153.0 x64: Current Version

You will be returned to the archive panel with test-one.zip. Panel title will show ArcLite:zip:test-one.zip If you press Ctrl+PgUp to exit this archive, you will be returned to the %TEMP% directory, and the cursor will be on test-one.zip. The file test-two.zip is not deleted automatically, which is correct. If you use -r -x test-two.zip to open the second archive, it is deleted on close, which is also correct. I also tried out the plugin prefix arc: with both -r and -x, and it is also working correctly.

Everything looks good!

@rohitab
Copy link
Contributor Author

rohitab commented May 26, 2023

@w17 Is it possible for -x option to work for the first opened archive? Currently, if you specify -r -x the first file is not deleted on close.

Test Case

As an example, in this test case, test-two.zip gets deleted, but test-one.zip does not get deleted.

cd "%TEMP%"
copy /Y "%FARHOME%\Plugins\FarColorer\base\common.zip" test-one.zip
goto:test-one.zip
copy /Y test-one.zip test-two.zip
lua:Plugin.Command("65642111-AA69-4B84-B4B8-9249579EC4FA", "-r -x test-one.zip")
lua:Plugin.Command("65642111-AA69-4B84-B4B8-9249579EC4FA", "-r -x test-two.zip")
lua:Keys("CtrlPgUp")

Open Panel Flags

Is it possible to have the new flags OPIF_DELETEDIRONCLOSE and OPIF_DELETEFILEONCLOSE that you added in 3776d9c always work? Something like the following at the end of FileList::PopPlugin function in far/filelist.cpp

	// Delete Host file (and containing directory), if specified
	if (!(cached_Flags & OPIF_REALNAMES) && !cached_hostfile.empty())
	{
		if (cached_Flags & OPIF_DELETEDIRONCLOSE)
		{
			DeleteFileWithFolder(cached_hostfile);
		}
		else if (cached_Flags & OPIF_DELETEFILEONCLOSE)
		{
			std::ignore = os::fs::delete_file(cached_hostfile);
		}
	}

I tried the above code and it seems to be working without any issues so far. The first archive gets deleted as well now. The above code is just proof-of-concept. I'm in no way suggesting that you use it.

I think this is a good feature to have, especially with the new flags, since it should not affect any plugins that don't use them. This would allow using commands like arc:-x %TEMP%\example.zip to view the file, and have it automatically deleted on close.

@w17
Copy link
Contributor

w17 commented May 26, 2023

I think it is possible (and good idea).

@MKadaner
Copy link
Contributor

@w17, I apologize, I have not closely followed the proceedings here, however...

It looks like you added a new user-facing functionality to the arclite plugin. New command line options it seems. Is it worth updating plugin help? I can do the actual writing, but I would need some kind of informal description or explanation. What do you think?

@w17
Copy link
Contributor

w17 commented May 28, 2023

It is still experimental.
And probably there are some unexpected errors/side effectects (according recently forum reports).
Hepl update is required but a bit later...

@w17
Copy link
Contributor

w17 commented May 29, 2023

@rohitab, could you check build 6155?

@rohitab
Copy link
Contributor Author

rohitab commented May 30, 2023

@w17, I just tested version 3.0.6156.0 x64. It's working great, except for an issue with the Temporary panel. For all other panels, the file is deleted successfully, so thank you for fixing that.

Issue

The file does not get deleted if it is opened from a Temporary panel.

Test Case

Here is an updated test case that reproduces the issue. You will see the message ERROR: File not Deleted and the file will still be visible in the Temporary panel.

cd "%TEMP%"
copy /Y "%FARHOME%\Plugins\FarColorer\base\common.zip" test-one.zip
goto:test-one.zip
tmp:<echo %TEMP%\test-one.zip
arc:-r -x test-one.zip
lua:Keys("CtrlPgUp")
lua:=mf.fexist("test-one.zip") and "ERROR: File not Deleted" or "OK: File Deleted"

The results are the same if you use Plugin.Command instead of arc: prefix.

OPIF_REALNAMES

Just for testing, I moved the check for OPIF_REALNAMES from line 5673 to line 5678, so that the check is performed after checking that !new_way is true.

FarManager/far/filelist.cpp

Lines 5673 to 5681 in edadb6c

if (!(m_CachedOpenPanelInfo.Flags & OPIF_REALNAMES) && !CurPlugin->m_HostFile.empty()) // remove previous plugin host-file/directory
{
if (cached_hostfile == CurPlugin->m_HostFile) // just in case
{
const bool new_way = (cached_Flags & (OPIF_RECURSIVEPANEL | OPIF_DELETEFILEONCLOSE | OPIF_DELETEDIRONCLOSE)) != 0;
if (!new_way || (cached_Flags & OPIF_DELETEDIRONCLOSE) != 0) del_mode = 'd';
else if (new_way && (cached_Flags & OPIF_DELETEFILEONCLOSE) != 0) del_mode = 'f';
}
}

After making this change and running the above test, I see the message OK: File Deleted, and the Temporary panel is empty, as expected.

@w17
Copy link
Contributor

w17 commented May 31, 2023

6157

@rohitab
Copy link
Contributor Author

rohitab commented Jun 1, 2023

Perfect! Just tried version 3.0.6157.0 x64. Everything is working fine now. Thank you.

@w17
Copy link
Contributor

w17 commented Jun 8, 2023

@MKadaner

I can do the actual writing, but I would need some kind of informal description or explanation.

https://forum.farmanager.com/viewtopic.php?p=175151#p175151

@MKadaner
Copy link
Contributor

MKadaner commented Jun 9, 2023

Thanks. I'll find time. Meanwhile, #703.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants