-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
[SHELL32] brfolder.cpp: Code formatting and refactoring #6909
Conversation
621e996
to
e869519
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
I'm going to review this during this week. |
7884f37
to
73b5fbd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see any obvious problems here but I would strongly suggest the following, in order to ease diffs and review:
- in the first commit, do all the whitespace/comments/brackets/code-style changes ONLY;
- in the second commit, do all the refactoring (renamings of structs, variables, and usage of COM/ATL helpers etc.)
JIRA issue: CORE-17340
Modernize source code. Improve readability and reduce confusing. JIRA issue: CORE-17340 - Rename browse_info as BrsFolder. - Rename TV_ITEMDATA as BrsItemData. - Rename InsertTreeViewItem as BrsFolder_InsertItem. - Rename InitializeTreeView as BrsFolder_InitTreeView. - Rename GetIcon as BrsFolder_GetIcon. - Rename GetNormalAndSelectedIcons as BrsFolder_GetIconPair. - Rename GetName as BrsFolder_GetName. - Fix some confusing and/or non-standard variable names. - Use ATL to simplify logics.
7ba87cb
to
1cd85e6
Compare
@HBelusca Done. |
and rename BrsItemData as BrItemData to reduce binary size of Debug version. JIRA issue: CORE-17340
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved. Please merge the 3 commits in unsquashed form. Makes sense to wait for Hermes review to complete.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks nice this way. The commits need to be merged separately (NOT squashed)
Follow-up of #6909 (af03438). Fix assertions and exceptions of CComPtr and CComHeapPtr. JIRA issue: CORE-17340 - Don't use AddRef, Release, and Attach methods against CComPtr template class. Simply use assignment. - Fix the 3rd parameter of three IShellFolder::GetAttributesOf method calls. It had referenced CHeapPtr::operator&.
Purpose
Modernize source code. Improve readability and reduce confusing.
JIRA issue: CORE-17340
Proposed change
brsfolder.cpp
asbrfolder.cpp
.browse_info
asBrFolder
.TV_ITEMDATA
asBrItemData
.InsertTreeViewItem
asBrFolder_InsertItem
.InitializeTreeView
asBrFolder_InitTreeView
.GetIcon
asBrFolder_GetIcon
.GetNormalAndSelectedIcons
asBrFolder_GetIconPair
.GetName
asBrFolder_GetName
.TODO