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

Added a --progress option for snazzer-receive. #40

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Added a --progress option for snazzer-receive. #40

wants to merge 1 commit into from

Conversation

DL6AKU
Copy link

@DL6AKU DL6AKU commented Oct 27, 2016

Fixes #39

@florianjacob
Copy link
Collaborator

Good idea!

I'm thinking about whether the explicit command line option is really needed though - maybe we could turn progress metering automatically if snazzer is run interactively, i.e. stderr is a tty with a user in front of it? In fish there's isatty but I don't know how to do that reliably in bash. 😉

@florianjacob
Copy link
Collaborator

Seems like checking that when run in a terminal via ssh is not that trivial in bash, though…

@DL6AKU
Copy link
Author

DL6AKU commented Oct 27, 2016

We could show the progress by default if pv is installed. That would actually make for a simpler patch. I did not want to go so far though with my patch. Such a default behaviour would have to be decided by the project maintainer I guess (@csirac2).

@csirac2
Copy link
Owner

csirac2 commented Oct 27, 2016

Great idea! I would love to see receive progress indicator by default, ssh does look fiddly to detect though.

My only concern with the code is that I would like to avoid eval.

@DL6AKU
Copy link
Author

DL6AKU commented Oct 27, 2016

Afaik it wouldn't hurt ssh if it was enabled for ssh as well (so we skip the ssh detection), though I did not look into the ssh option just now.

I do not know about bash magic enough to avoid the eval. I tried without, but then you get a double pipe or pv: | not found errors. Or you get more nested ifs.

I can look into expansion of the patch tomorrow.

@florianjacob
Copy link
Collaborator

To be more explicit: If possible, the progress meter should show automatically in three cases:

  • virtual terminals used interactively
  • terminal emulators used interactively
  • ssh sessions used interactively

It shouldn't show if snazzer is started from inside a shell script, a cron job or a systemd timer, or when the stderr is redirected to a file or something else that is not a tty.

References: http://stackoverflow.com/questions/911168/how-to-detect-if-my-shell-script-is-running-through-a-pipe
Maybe helpful: http://tldp.org/LDP/abs/html/intandnonint.html

@florianjacob
Copy link
Collaborator

Regarding the conditional pipeline issue, this looks like a solution that does not need eval or another if level: http://unix.stackexchange.com/questions/38310/conditional-pipeline

@csirac2
Copy link
Owner

csirac2 commented Oct 28, 2016

I'm not sure I'll get it to it this weekend, and I'm in a crunch at work for the next week, but I'll try to take a look next weekend.

@DL6AKU
Copy link
Author

DL6AKU commented Dec 12, 2016

Just FYI, I am no no longer using snazzer, so I won't be actively working towards a patch.

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.

None yet

3 participants