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

move special_folder into rsutils #12417

Merged
merged 3 commits into from
Nov 20, 2023

Conversation

maloel
Copy link
Contributor

@maloel maloel commented Nov 16, 2023

First step of persistent DDS settings.

Moved it to rsutils (first commit).
Made behavior consistent for all cases, and removed Windows warnings about insecure function. (second commit, so you can see the actual changes)

Tracked on [LRS-960]

@maloel maloel requested a review from OhadMeir November 16, 2023 06:28
@maloel
Copy link
Contributor Author

maloel commented Nov 19, 2023

Should I just include the followup in this PR? :)

CHECK_HR( SHGetFolderPathA( NULL, CSIDL_PERSONAL, NULL, 0, path ) );
res = path;
res += "\\";
return res;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like the return here, it is not obvious.
If this is a special case that needs different handling then do it in an if like the temp folder

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh, I see the code was moved from another location. Still, consider changing...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed.

namespace os {


std::string get_special_folder( special_folder f )
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rename get_special_folder_path, it does not return a descriptor or such like.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It returns a "folder". I don't know if I understand the connection to a descriptor.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It returns a string that represents the folder path in the file system. It does not return a folder "object".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But there is no such object. Why does it have to be an object? A folder is represented by a path which is represented by a string, almost universally.

@maloel maloel merged commit 11878e6 into IntelRealSense:development Nov 20, 2023
16 checks passed
@maloel maloel deleted the special-folder branch November 20, 2023 12:22
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