search

dotnet / pinvoke

A library containing all P/Invoke code so you don't have to import it every time. Maintained and updated to support the latest Windows OS.
MIT License
2.12k stars 222 forks source link

Shell32 library #247

Closed arlm closed 8 years ago

arlm commented 8 years ago

I added KnowFolders related funcitions and constants

vbfox commented 8 years ago

Except for the IntPtr -> pointer the rest LGTM

AArnott commented 8 years ago

Why is your src/Shell32.Desktop/Shell32.exports.txt file empty? It should have a bunch of function names in it. The AddNewLibrary.ps1 script generally takes care of this for you. Did it malfunction? Could you perhaps try directly running CreateExportsTxtFile.ps1 to see if it can create it properly?

arlm commented 8 years ago

Strange. It seems that it failed. But the AddNewLibrary run.

File C:...\pinvoke\templates\CreateExportsTxtFile.ps1 cannot be loaded because running scripts is disabled on this system. For more information, see about_Execution_Policies at http://go.microsoft.com/fwlink/?LinkID=135170.

arlm commented 8 years ago

Is there any other way to populate it?

AArnott commented 8 years ago

You could run this powershell command to unblock it, I think: set-executionpolicy remotesigned

AArnott commented 8 years ago

I'm working on this PR now to resolve some issues we've talked about. Also, the ILFree method is missing, which evidently is a requirement when anyone calls SHGetKnownFolderIDList

AArnott commented 8 years ago

Well, I've come away convinced that pointers were definitely important to retain here. The rules for these functions and how to free the memory they allocate are diverse, and there's no way the .NET Marshaler could handle it all automatically. It must be done with native pointers (or IntPtr) rather than marshaled so that the original pointer is available to free later. Helper methods can help keep things sane for the caller though. I'm pretty sure there were memory leaks in the original PR.

AArnott commented 8 years ago

SetLastError=true is inappropriate, because these functions don't document to use it. Also you seem to be treating error codes as if they were Win32 errors by throwing Win32Exception, which is inappropriate because these functions return HRESULTs.

AArnott commented 8 years ago

@vbfox please consider looking at arlm/pinvoke#2 to review the changes I'm proposing to this PR.

AArnott commented 8 years ago

But before we can merge this PR, #248 needs to be fixed to avoid introducing a bunch of build warnings.

vbfox commented 8 years ago

@AArnott arlm/pinvoke#2 LGTM, nice that we can have tests for that. I don't know if the manual methods to change / get the value under an SHITEMID will be useful to anyone but the code LGTM.

(Make me think that I could create a NuGet version of my answer on how to open explorer with files selected http://stackoverflow.com/a/3578581/46594 now that we are adding shell functions)

AArnott commented 8 years ago

Thanks. Yes, I meant to add tests for my SHITEMID accessor methods. But I also found in debugging that the buffer appears to not be intended for direct consumption, so I can probably just remove them all.

AArnott commented 8 years ago

I've removed the accessor methods and added doc comments warnings folks to always use pointers (or IntPtr) when handling these structs.

AArnott commented 8 years ago

Thanks for your contribution, @arlm!