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

Add quiet (-q|--quiet) mode to suppress Ctrl+D banner #39

Merged
merged 1 commit into from
Jan 5, 2021

Conversation

Daviey
Copy link
Contributor

@Daviey Daviey commented Jan 5, 2021

Previously, all recordings would display, "Press Ctrl+D to end
recording". This is helpful (& even desirable) on first usage.
However, it is annoying to have the banner in all the recordings.

This adds the ability to add -q|--quiet, which suppresses the banner
making cleaner recordings. (An empty string is outputted as the
application needs there to be something to begin recording)

Signed-off-by: Dave Walker (Daviey) [email protected]

@Daviey Daviey changed the title Add Quiet (-q|--quiet) to suppress Ctrl+D banner Add quiet (-q|--quiet) mode to suppress Ctrl+D banner Jan 5, 2021
Copy link

@rockstar rockstar left a comment

Choose a reason for hiding this comment

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

I would like to see this merged, please.

@@ -105,7 +105,11 @@ fn main() -> Result<()> {
println!("Recording window id: {}", win_id);
}
}
println!("Press Ctrl+D to end recording");
if args.is_present("quiet") {
println!();
Copy link

Choose a reason for hiding this comment

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

Do we need to println!() at all? Do we need the extra \n?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I tried to express this in the commit message, "an empty string is outputted as the
application needs there to be something to begin recording".

Without it, there is no recording and the following output:

$ t-rec -q
[src/common/identify_transparency.rs:24] margin.top = 0
[src/common/identify_transparency.rs:34] margin.bottom = 0
[src/common/identify_transparency.rs:44] margin.left = 0
[src/common/identify_transparency.rs:54] margin.right = 0
$ echo $?
0

Copy link
Owner

Choose a reason for hiding this comment

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

Instead of println!(); you could use print!("{esc}[H", esc = 27 as char); that seems to work too.

The dbg output might be something that I forgot to remove.. in src/common/identify_transparency.rs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're the boss, but that doesn't seem to work for me. Exhibits the same behaviour as no print at all. println!("{esc}[H", esc = 27 as char); works, would you prefer that to println!()?

Copy link
Owner

Choose a reason for hiding this comment

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

hm, interesting. What happens in your case? The recording does not start at all?

Copy link
Owner

Choose a reason for hiding this comment

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

I'm fine with the empty line for now, I don't mind. A future goal is to have a little count down indicating that the recording will start in 3, 2, 1 - <clear> So that a user gets feedback that it worked, and has 3 seconds to prepare and the recording will start on a clear screen.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hm, interesting. What happens in your case? The recording does not start at all?

Nested t-rec didn't work very well... but yes. No attempt to record with that, just exit out (with the debug):

t-rec

Copy link
Contributor Author

Choose a reason for hiding this comment

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

println!(); doesn't actually output a new line on the recorded screen:
t-rec

Copy link
Owner

Choose a reason for hiding this comment

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

Good to know, anyway the other issue that the recording does not start then is something I will dig into.
But as far as this PR is concerned the new line is fine for me.

CHANGELOG.md Outdated Show resolved Hide resolved
Previously, all recordings would display, "Press Ctrl+D to end
recording".  This is helpful (& even desirable) on first usage.
However, it is annoying to have the banner in all the recordings.

This adds the ability to add -q|--quiet, which suppresses the banner
making cleaner recordings.  (An empty string is outputted as the
application needs there to be something to begin recording)

Signed-off-by: Dave Walker (Daviey) <[email protected]>
@sassman
Copy link
Owner

sassman commented Jan 5, 2021

Thanks a lot for this contribution, always good to learn what users are concerned with.

BTW: there is this little conversation about the configurability of the frame rate in #36 and #28 - I would be very happy to get feedback if the suggested solution would be helpful or not.

@sassman sassman merged commit b4644c7 into sassman:main Jan 5, 2021
@Daviey
Copy link
Contributor Author

Daviey commented Jan 20, 2021

(@sassman I did look at those issues, but I didn't comment as I didn't have anything valuable to add. Thanks again!)

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.

3 participants