Screenshot enhancement : output filename specification in silent mode

[Shisui]

This enhancement allows the user to use parametric filenames. It offers more flexibility than automatic file numbering in silent mode (screenshot -s). Recognized parameters include those parsed by strftime() and additional ones related to the screenshot size and output type.

For example :

screenshot -s "screenshot \$wx\$h %m-%d-%y %H:%M:%S.png"

Produces a file named "screenshot 1024x768 04-14-2010 21:30:32.png" in the current directory.

(the backslashes are required if the shell interpret $ as variables delimiter)

Known parameters (in addition of strftime() format parameters) are $w and $h for the width and height of the screenshot, and $t for the format type of the produced file (e.g: PNG, BMP, ..)

Parameters are also parsed in window mode.

[axeld]

To be honest, I find this pretty much useless and overkill. But maybe that's just me, other opinions welcome.

[Shisui]

It's useful for scripting, for example, the user has a greater control over the generated filename.

[axeld]

Besides that there is very limited use of scripting Screenshot, most of the data is available in the shell already, so you could easily build such names there directly, without having special support in Screenshot.

The only thing that would make sense to add to Screenshot in this regard is being able to change the filename in the first place, though.

The patch itself looks good, I just don't see much use in the functionality it adds.

BTW, "static" is a qualifier like "virtual"; it should be indented in the same way. Also, '{' goes to the end of the previous line, the only exceptions are functions.

[Shisui]

The patch also enables filename/directory selection in silent mode. Even with this, if someone creates a desktop shortcut to make a screenshot, the only possibilities he gets are creating screenshot#.png in the prefered directory, or a fixed filename in a specified directory. I don't say that adding a timestamp in the filename is a everyday use of Screenshot, but it can be useful, it's a "plus" I believe. Anyway, I updated the patch according to the coding guidelines mistakes you raised, thanks :)

[axeld]

As I said, the filename/directory specification definitely makes sense.

The rest, however, does not IMO, since you can create the very same name using bash without any extra feature in Screenshot, and that also includes the timestamp (which is a bit superfluous, since we have last modified and creation time).

[axeld]

BTW thanks for the updated patch. However, there are still lots of coding style issues with it, for example:

(line 140)

	void* item; 
	for ( int32 i = 0; ; i++ ) { 
		item = fFilenameParameters.ItemAt(i); 

Since "item" is not used outside the loop, why not declare it inside? There shouldn't be spaces between ().

(line 447)

	.Add(new BButton("", TR("Options"), new 
BMessage(kShowOptions))) 

What happened here?

(line 819 and others)

ScreenshotWindow::_AddFileParameterRule(FileParameter index, 
                   const char* name) 

Only one tab for indentation in this case.

(line 849)

	case kImageWidth: 
		rule->replacementValue << 
			screenshotParent->fScreenshot->Bounds().IntegerWidth() + 1; 

The operator always goes to the start of the next line.

It also doesn't fix the '{' style everywhere. Please consult ​http://www.haiku-os.org/development/coding-guidelines for more information.

[Shisui]

(line 447) What happened here?

Good question ... Mistyping or vim line wrap rule I think, I missed it while reading the patch.

I'm not yet used to Haiku coding guidelines, I just discovered checkstyle.py, It already gives me headaches! :) I tried to fix what it reported.

[Shisui]

Patch implementing the parametric filenames functionality

[axeld]

Adapting to a coding style always takes a bit of time, knowing it is a good start, though :-)

Anyway, would you mind doing an alternative version of your patch that only implements adding support for a different file name? That part of the patch is definitely an improvement, while I personally wouldn't want to apply the rest.

[Shisui]

In fact the soft patch was already done, given your comments, but I forget to send it, sorry, here it is.

[Shisui]

Allows the user to specify an output filename in silent mode.

[axeld]

Great, thanks! I've applied it in hrev36312.

I've made a few changes, and here is why:

• There is no reason to make the filename parameter a BString, so I changed it to a const char*.
• Furthermore, a BString is empty by default, so there is no need to set it to "" manually.
• Also, it should have been "const BString&" in order to save an extra copy when passing the object around.

In any case, I keep this ticket open for a while to collect other opinions on the matter.

[Shisui]

Thanks for those advices :) (I can't see why I didn't keep a const char* for this basic use of strings..)

[Wim]

I tend to agree with Axel that adding parametric filename functionality doesn't make sense.

[Wim]

Closing the ticket.

In addition, the screenshotEnhancement-parametricFilename.patch can't be applied cleanly anyway with the latest screenshot update.