Mark Jansen

This can be different per use case. If you need the buffer just to read from it, use GetString(), if you need the buffer to modify it, use GetBuffer() / ReleaseBuffer(). If you are going to pass t...

This can be different per use case.
If you need the buffer just to read from it, use GetString(), if you need the buffer to modify it, use GetBuffer() / ReleaseBuffer().

If you are going to pass the data to a control that holds on to it (lParam etc) you probably dont want to use a CString at all.

A generic note for CString: If you just want to access the raw string pointer (to read!), use GetString() instead of GetBuffer().

A generic note for CString: If you just want to access the raw string pointer (to read!), use GetString() instead of GetBuffer().

Samuel Serapion: You said you were working on a new patch, is the latest version attached here or do you have a new revision?

Samuel Serapion: You said you were working on a new patch, is the latest version attached here or do you have a new revision?

  • More
  • CR-120
  • finished reviewing
  • More
  • CR-122
  • finished reviewing
Jira cut off the url (already gave it in irc): https://git.reactos.org/?p=reactos.git;a=blob;f=reactos/dll/win32/browseui/explorerband.cpp;hb=6f6999647ab23b1ee53550b6f6196c912b03334a#l57

Jira cut off the url (already gave it in irc): https://git.reactos.org/?p=reactos.git;a=blob;f=reactos/dll/win32/browseui/explorerband.cpp;hb=6f6999647ab23b1ee53550b6f6196c912b03334a#l57

As you already found out, and what we overlooked was: The TBBUTTON struct treats this as opaque data, e.g. it does do nothing but store the raw value you pass it. (in our case, a pointer) So when f...

As you already found out, and what we overlooked was: The TBBUTTON struct treats this as opaque data, e.g. it does do nothing but store the raw value you pass it. (in our case, a pointer)
So when freeing this data, the pidl becomes invalid, and the button can no longer use it.

You are probably looking for _IlIsDesktop

You are probably looking for _IlIsDesktop

Yes. But remember: When the CComHeapPtr is not inside the for loop, it needs to be freed each iteration as well!

Yes.
But remember: When the CComHeapPtr is not inside the for loop, it needs to be freed each iteration as well!

Or route allocations trough a generic macro / function, which you can use to switch between the 2 https://code.reactos.org/static/olpro3/2static/images/wiki/icons/emoticons/smile.gif

Or route allocations trough a generic macro / function, which you can use to switch between the 2

Update Sequence Array

Update Sequence Array

  • More
  • CR-119
  • resumed reviewing
Yes sorry, I meant CWindow. that is the ATL version, it's simply a thin wrapper around an HWND, allows some convenience functions.

Yes sorry, I meant CWindow.
that is the ATL version, it's simply a thin wrapper around an HWND, allows some convenience functions.

The return here is related to how the window message is processed further, not if the function is successful. So in this case, using FAILED_UNEXPECTEDLY will log an error to the debug output, showi...

The return here is related to how the window message is processed further, not if the function is successful.
So in this case, using FAILED_UNEXPECTEDLY will log an error to the debug output, showing that something unexpected happened.

Create a string table entry for it, Load that (for example with CString title; title.LoadString(YOUR_RESOURCE_ID), and use the loaded string in the BROWSEINFO. This allows us to localize the string...

Create a string table entry for it,
Load that (for example with CString title; title.LoadString(YOUR_RESOURCE_ID),
and use the loaded string in the BROWSEINFO.
This allows us to localize the string by adding translations in the resource file.

see for an example: https://git.reactos.org/?p=reactos.git&a=search&h=HEAD&st=grep&sr=1&s=IDS_ALLPICTUREFILES

  • More
  • CR-118
  • finished reviewing
  • More
  • CR-115
  • finished reviewing
  • More
  • CR-92
  • finished reviewing
I just noticed that my own shell extension also does this.... copypasta ftw

I just noticed that my own shell extension also does this.... copypasta ftw

in that case, also use MAKEINTRESOURCEW

in that case, also use MAKEINTRESOURCEW

GCC when compiling for linux has a different view of long, but we are not compiling for linux https://code.reactos.org/static/olpro3/2static/images/wiki/icons/emoticons/wink.gif

GCC when compiling for linux has a different view of long,
but we are not compiling for linux

InitializeSFB takes ownership?

InitializeSFB takes ownership?

We should probably localize this?

We should probably localize this?

CComHeapPtr<ITEMIDLIST>

CComHeapPtr<ITEMIDLIST>

We just miss the macro's BEGIN_CATEGORY_MAP, IMPLEMENTED_CATEGORY and END_CATEGORY_MAP. If we have those, atl should be able to do this. (see AtlRegisterClassCategoriesHelper)

We just miss the macro's BEGIN_CATEGORY_MAP, IMPLEMENTED_CATEGORY and END_CATEGORY_MAP.
If we have those, atl should be able to do this. (see AtlRegisterClassCategoriesHelper)

winmm? are you sure you need all these?

winmm?
are you sure you need all these?

if you include the header files in this, they will also show up in visual studio.

if you include the header files in this, they will also show up in visual studio.

including just ${REACTOS_SOURCE_DIR} seems odd.

including just ${REACTOS_SOURCE_DIR} seems odd.

  • More
  • CR-113
  • finished reviewing
Should this also return S_OK if m_hWndTb is null?

Should this also return S_OK if m_hWndTb is null?