Skip to content

Solved undo deleted item.#486

Closed
flaviu22 wants to merge 2 commits intoderceg:masterfrom
flaviu22:master
Closed

Solved undo deleted item.#486
flaviu22 wants to merge 2 commits intoderceg:masterfrom
flaviu22:master

Conversation

@flaviu22
Copy link
Contributor

I have solved the missing feature option -> Undo deleted item.

@derceg
Copy link
Owner

derceg commented Sep 30, 2024

Thanks for this. As a start, could you reduce this down to only the necessary changes. At the moment, this pull request includes both unrelated changes (e.g. adding a Ctrl+Q accelerator) as well as unnecessary changes (e.g. formatting changes, use of CComPtr instead of wil::com_ptr_nothrow, changes to the resource headers, changes to ShellBrowserImpl.cpp and ShellTreeView.cpp).

@flaviu22
Copy link
Contributor Author

flaviu22 commented Sep 30, 2024

Ok, I will try to let only necessary changes, so, I should add Ctrl+Q shortcut to another pull request? Will you accept this new pull request? I am asking this to know if I should prepare another pull request.

Of course, this is not a condition, I will filter the current pull request with what it needs, no problem.

@flaviu22
Copy link
Contributor Author

flaviu22 commented Oct 1, 2024

See the following method:

HRESULT FileOperations::Undelete(const PCIDLIST_ABSOLUTE &pidl)
{
	wil::com_ptr_nothrow<IShellFolder> pDesktop{};
	HRESULT hr = SHGetDesktopFolder(&pDesktop);
	if (SUCCEEDED(hr) && pDesktop)
	{
//		CComHeapPtr<ITEMIDLIST_ABSOLUTE> pidlbin{};
		wil::com_ptr_nothrow<ITEMIDLIST_ABSOLUTE> pidlbin;
		hr = SHGetKnownFolderIDList(FOLDERID_RecycleBinFolder, KF_FLAG_DEFAULT, NULL, &pidlbin);
		if (SUCCEEDED(hr) && pidlbin)
		{
			wil::com_ptr_nothrow<IShellFolder> pShellFolder{};
			hr = pDesktop->BindToObject(pidlbin.get(), nullptr, IID_PPV_ARGS(&pShellFolder));
			if (SUCCEEDED(hr) && pShellFolder)
			{
				wil::com_ptr_nothrow<IEnumIDList> pEnum{};
				hr = pShellFolder->EnumObjects(NULL, SHCONTF_FOLDERS | SHCONTF_NONFOLDERS, &pEnum);
				if (SUCCEEDED(hr) && pEnum)
				{
					ULONG fetched{};
					for (PITEMID_CHILD pidChild{}; pEnum->Next(1, &pidChild, &fetched) == S_OK; pidChild = nullptr)
					{
						const auto pidlRelative = ILFindLastID(static_cast<PCUIDLIST_RELATIVE>(pidl));
						hr = pShellFolder->CompareIDs(SHCIDS_CANONICALONLY, pidlRelative, pidChild);
						if (0 == static_cast<short>(HRESULT_CODE(hr)))
						{
							hr = PerformUndeleting(pShellFolder, pidChild);
							break;
						}
					}
				}
			}
		}
	}

	return hr;
}

I don't think I can remove CComHeapPtr from this method, I get:

30>C:\Project\explorerplusplus\vcpkg_installed\x64-windows-static\x64-windows-static\include\wil\com.h(263,20): error C2039: 'Release': is not a member of '_ITEMIDLIST_ABSOLUTE'
if I use wil::com_ptr_nothrow instead of CComHeapPtr for pidlbin.

@flaviu22
Copy link
Contributor Author

flaviu22 commented Oct 2, 2024

I'll come out with another pull request.

@derceg
Copy link
Owner

derceg commented Nov 16, 2024

Ok, I will try to let only necessary changes, so, I should add Ctrl+Q shortcut to another pull request? Will you accept this new pull request? I am asking this to know if I should prepare another pull request.

Pull requests for new shortcuts are fine in general. In this specific case, I'm not really a fan of adding Ctrl+Q as a default shortcut, since Q is right next to W, making it easy to accidentally close the application, instead of closing the current tab. I've had that happen to me in other applications, for example. There's ongoing work (tracked in #476) to allow keyboard shortcuts to be customised, so once that's done, you could add Ctrl+Q as a user shortcut, without having to modify the code.

@flaviu22
Copy link
Contributor Author

So, the undo deelted changes will be rejected? Have you seen I removed that Ctrl+Q shortcut? That's why I named - clean code the pull request.

@derceg
Copy link
Owner

derceg commented Nov 18, 2024

Sorry, I meant I wasn't really in favor of adding a Ctrl+Q shortcut, specifically. I'm still planning on reviewing the changes in the updated pull request.

@flaviu22
Copy link
Contributor Author

flaviu22 commented Dec 3, 2024

Another option will be to have Ctrl+Shift+W as Quit accelerator.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants