[Screenshot] Artwork folder is read-only (easy)

[diver]

Since Artwork folder lives in a package now, the Screenshot option to save there should either be removed or non-packaged version of Artwork folder should be added.

[pulkomandy]

Fixed in hrev48507.

[diver]

Still unable to save there, probably because Screenshot doesn't create B_USER_NONPACKAGED_DATA_DIRECTORY folder on save.

[gbl08ma]

My patch fixes the issue by creating the artwork folder (or any other folder that might be added to the folder selection dropdown in the future) if it doesn't exist.

I have also noticed that when there's a problem saving the file, it just fails silently. I would expect at least a message box informing about the failure, but instead the Screenshot tool just stays open. If one doesn't remember the usual behavior of closing on success, one may be fooled by thinking the file saved sucessfully. But that's probably a subject for another issue.

[pulkomandy]

Hi, The patch looks good. But there are possible improvements:

1) From the coding guidelines:

Variables start with lowercase letters and use interCapsFormatting.

So "direntry" is not correct. It should be dirEntry or even better, directoryEntry.

2) Testing that the directory exists is not enough. It could be a file or a broken symlink, and in these cases the code below will fail anyway. You could use BEntry.IsDirectory() instead.

3) Creating a temporary BDirectory just for calling CreateDirectory with an absolute path is not very useful. You could use the C function create_directory instead.

[gbl08ma]

Replying to pulkomandy:

Hi, The patch looks good. But there are possible improvements:

1) From the coding guidelines:

Variables start with lowercase letters and use interCapsFormatting.

Oops, I saw this guideline and yet managed to break it.

2) Testing that the directory exists is not enough. It could be a file or a broken symlink, and in these cases the code below will fail anyway. You could use BEntry.IsDirectory() instead.

I read the docs on api.haiku-os.org and I'm still a bit confused over what BDirectory, BEntry, BFile and BPath do, and managed to miss IsDirectory because it comes from BStatable (which in hindsight makes perfect sense, I should have clicked to see the list of all members).

3) Creating a temporary BDirectory just for calling CreateDirectory with an absolute path is not very useful. You could use the C function create_directory instead.

This came from an attempt to stick to object-oriented APIs, and because I couldn't find docs pointing me to create_directory. I looked for a static method in BDirectory to do this (since I agree it doesn't make any sense to create a BDirectory for this) but forgot to look for methods outside of the class. I'm still new to the Haiku API organization style, so forgive me :)

I'll redo my patch (eventually after some minor git wrestling) and reupload.

[gbl08ma]

I'm not sure if I did the right thing with IsDirectory, but at least now execution no longer continues from there if the entry exists but isn't a directory. In the future this may be extended to give the user a better error message.

[pulkomandy]

Applied in hrev50530, thanks! Feel free to continue with the improved error handling and user alerts, or something else as you wish :)