-
Notifications
You must be signed in to change notification settings - Fork 7
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 delete history item subcommand to results command #69
Conversation
Add subcommand to delete given item in history including results and log file.
- Move --no-color option to base `ipa` command and make available to all subcommands. - Move --history-log to results command and make available to all results subcommands.
If no subcommand is provided invoke show with default value of 1.
lines = f.readlines() | ||
history = lines.pop(len(lines) - item) | ||
f.seek(0) | ||
f.write(''.join(lines)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should flush the file before calling truncate. The Python documentation is not explicit about the flush, it is pointed out in the Python doc then if one calls truncate from the os module flush and sync are required ahead of time. Since we do not know the internals lets at least flush the content of the file out of the python buffers before truncating the file size.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think calling flush before truncate does anything. But it doesn't hurt anything either.
But looking at the cpython unit tests there's no use of explicit flush. https://github.com/python/cpython/blob/a5af6e1af77ee0f9294c5776478a9c24d9fbab94/Lib/test/test_fileio.py#L487
On close the file is flushed.
ipa/scripts/cli.py
Outdated
try: | ||
with open(history_log, 'r') as f: | ||
lines = f.readlines() | ||
lines.reverse() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Count from the bottom and skip the reverse
And get history item from bottom of list instead of reversing.
All steps are split into subsequent commits to help with review.
ipa results
still invokes default show latest entry.Resolves #69