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

Remove open system call for Darwin #4

Merged
merged 2 commits into from Sep 1, 2017
Merged

Conversation

eren
Copy link
Contributor

@eren eren commented Sep 1, 2017

Automatically opening a torrent via system call is an intrusive
operation. Tors is a torrent search utility and it shouldn't assume that
the user has a torrent client installed. It should be up to the user
whether to open it or not.

Resolves #3

Automatically opening a torrent via system call is an intrusive
operation. Tors is a torrent search utility and it shouldn't assume that
the user has a torrent client installed. It should be up to the user
whether to open it or not.

Resolves murat#3
@murat
Copy link
Owner

murat commented Sep 1, 2017

@eren maybe you are right. But I was planned make an option for open or don't open. Can you make this?

@eren
Copy link
Contributor Author

eren commented Sep 1, 2017

Yeah. I can send a PR for adding an option but it shouldn't be a blocker for this. It should be a default behavior not to open files.

@murat
Copy link
Owner

murat commented Sep 1, 2017

Yep we have same thoughts. I'm waiting your pr.

- Add -o/--open parameter on Darwin platform which defaults to false
- Refactor TorS::Search to accept a block on initialization stage
- Handle exception better on download by changing ensure statement to else.
  Ensure block always runs regardless of the error. We want to run this block
  if the download is successful.

Resolves murat#3
@murat murat merged commit 4b643df into murat:master Sep 1, 2017
@murat
Copy link
Owner

murat commented Sep 1, 2017

Thanks very much @eren

Best regard!

@eren eren deleted the remove-open-call branch September 2, 2017 16:28
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

2 participants