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

Fix in click.edit() for fast editors #1050

Merged
merged 2 commits into from
Aug 8, 2020

Conversation

giovannipizzi
Copy link
Contributor

If a fast editor is used and the filesystem
only provides a resolution of 1s, click.edit()
returns None if the file is edited and saved very quickly.

This is a workaround that should work on all systems.

The added test would fail on my system before the fix.

@giovannipizzi
Copy link
Contributor Author

Also, I understand that this is the right approach ()merge into 6.x-maintenance, and you take care of merging into master?

@giovannipizzi
Copy link
Contributor Author

Sorry, I realised there were a couple of tests not passing, I close this PR to fix them and I open it again ASAP.

@giovannipizzi
Copy link
Contributor Author

Tests should pass now (at least they pass in my fork) so for me the PR is ready to be merged.

@giovannipizzi giovannipizzi reopened this Jun 20, 2018
@giovannipizzi
Copy link
Contributor Author

Hello, will this get a chance to be merged? Please let me know if I should change anything

@davidism davidism closed this Sep 23, 2018
@davidism davidism reopened this Sep 23, 2018
@davidism davidism changed the base branch from 6.x-maintenance to master September 23, 2018 01:47
@davidism
Copy link
Member

Retargeted this to master. This won't be in 7.0 but could be targeted at 7.1. Needs review before I can say for sure.

@russkel
Copy link
Contributor

russkel commented Nov 21, 2018

Am I wrong in thinking that this test will fail on machines that do not have sed? Of course we're talking about the disfigured step child Windows.

@giovannipizzi
Copy link
Contributor Author

@russkel only the test will fail if sed is not available, but this I hope shouldn't be a problem (click itself will not need sed)

@davidism davidism force-pushed the fix-fast-edit branch 2 times, most recently from 46ffab1 to c30c041 Compare August 8, 2020 17:57
@davidism davidism added this to the 8.0.0 milestone Aug 8, 2020
giovannipizzi and others added 2 commits August 8, 2020 10:59
If a fast editor is used and the filesystem only provides a resolution
of 1 second, edit() returns None if the file is edited and saved very
quickly.
use context manager for opening file
move encoding out of try block
use isinstance instead of type
@davidism davidism merged commit 97fde8b into pallets:master Aug 8, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 13, 2020
@giovannipizzi giovannipizzi deleted the fix-fast-edit branch December 2, 2020 20:59
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants