search

resurrecting-open-source-projects / scrot

SCReenshOT - command line screen capture utility
Other
511 stars 51 forks source link

`scrot 'test$'` creates test$ #270

Open guijan opened 1 year ago

guijan commented 1 year ago

Commit 2e33c4cc81f0f6eeec696d81f393f6a7f061c796 changed scrot's behavior.

In 7ee199c (the preceding commit), scrot 'test$' would create a file named test. 2e33c4cc81f0f6eeec696d81f393f6a7f061c796 creates a file named test$.

guijan commented 1 year ago

More information because this is a bit cryptic: scrot ignores the $ of invalid special strings in an output filename. However, ever since 2e33c4cc81f0f6eeec696d81f393f6a7f061c796, if the $ is the last character in the string, it isn't ignored.

N-R-K commented 1 year ago

That change was intentional. But since it ignores unknown specifier in normal cases (which I didn't consider at the time), I can see how this can be confusing.

We can change it to either go back to previous behavior where $ or \ as last char would be ignored, or we can output unknown specifiers literally (e.g instead of turning $x into x, output $x instead).

The behavior on unknown specifier is undocumented, so we are free to go either ways.

guijan commented 1 year ago

I'd like to hard error on unknown specifiers.

N-R-K commented 1 year ago

I'd like to hard error on unknown specifiers.

For specifiers, sure. I don't mind that.

But what about backslashes? The current behavior on backslashes is extremely confusing (and borderline incorrect) to say the least. The manpage states: 

       \n   A literal newline (ignored when used in the filename).

It says, \n ignored when used in filenames, but "ignored" here basically means "skipping" it altogether:

[scrot master]~> ./src/scrot 'test\n<-skipped'
[scrot master]~> ls test*
'test<-skipped'

It also just removes any backslashes for no real reason:

[scrot master]~> ./src/scrot 'test\<-removed' 
[scrot master]~> ls test*
'test<-removed'

So if you wanted an actual backslash, you'd need to use \\.

guijan commented 1 year ago

I guess ignoring unnecessary escapes would be what the typical software does. We can do that, and maybe print a warning over it. Or hard error too. I don't know.

Edit: you just reminded me of undocumented behavior in scrot, I don't like it, probably should make an issue about that.