Recover recordings after unexpected exit/crash #899
Conversation
|
Wow. This is very comprehensive 🙌🏻 |
Co-authored-by: Sindre Sorhus <[email protected]>
main/recording-history.js
Outdated
|
|
||
| const setCurrentRecording = ({ | ||
| filePath, | ||
| name = `New Recording ${moment().format('YYYY-MM-DD')} at ${moment().format('H.mm.ss')}`, |
There was a problem hiding this comment.
This is now used in 3 places. I would extract it to a util: https://github.com/wulkano/Kap/search?q=YYYY-MM-DD&unscoped_q=YYYY-MM-DD
There was a problem hiding this comment.
I think we should also make it H => HH. This is what macOS uses for screenshots.
main/recording-history.js
Outdated
| recording.filePath, | ||
| '-v', | ||
| 'error', | ||
| '-f', |
There was a problem hiding this comment.
I think we should start commenting ffmpeg flags as calls to FFmpeg are totally unreadable.
main/recording-history.js
Outdated
| 'Close', | ||
| { | ||
| label: 'Show in Finder', | ||
| action: () => shell.showItemInFolder(recording.filePath) |
There was a problem hiding this comment.
Same here an in many other places.
| action: () => shell.showItemInFolder(recording.filePath) | |
| action: () => { | |
| shell.showItemInFolder(recording.filePath); | |
| } |
main/recording-history.js
Outdated
| action: () => shell.showItemInFolder(recording.filePath) | ||
| }, | ||
| { | ||
| label: 'Open in Editor', |
There was a problem hiding this comment.
| label: 'Open in Editor', | |
| label: 'Show in Editor', |
main/recording-history.js
Outdated
| }); | ||
|
|
||
| return updateUi({ | ||
| message: 'The recording was repaired successfully', |
There was a problem hiding this comment.
| message: 'The recording was repaired successfully', | |
| message: 'The recording was successfully repaired.', |
main/recording-history.js
Outdated
| } | ||
|
|
||
| return updateUi({ | ||
| message: 'We were unable to repair the recording', |
There was a problem hiding this comment.
| message: 'We were unable to repair the recording', | |
| message: 'Kap was unable to repair the recording.', |
test/recording-history.js
Outdated
| t.deepEqual(recordingHistory.get('recordings'), [ | ||
| {filePath: incomplete, name: 'Incomplete', date: new Date().toISOString()} | ||
| ]); |
There was a problem hiding this comment.
While this is more verbose, it's also IMHO more readable, as each assertion value are on separate lines.
| t.deepEqual(recordingHistory.get('recordings'), [ | |
| {filePath: incomplete, name: 'Incomplete', date: new Date().toISOString()} | |
| ]); | |
| t.deepEqual( | |
| recordingHistory.get('recordings'), | |
| [ | |
| { | |
| filePath: incomplete, name: 'Incomplete', date: new Date().toISOString() | |
| } | |
| ] | |
| ); |
test/recording-history.js
Outdated
| t.is(recordingHistory.get('recordings').length, 0); | ||
| }); | ||
|
|
||
| test('checkForActiveRecording with unknown corrupt recording', async t => { |
There was a problem hiding this comment.
| test('checkForActiveRecording with unknown corrupt recording', async t => { | |
| test('`checkForActiveRecording()` with unknown corrupt recording', async t => { |
Just to make it clear it's a function. Applies in multiple places.
|
Can you fix the conflict? |
|
Resolved 👍 |
|
So awesome to see, thank you! |
Inspired by wulkano/aperture-node#13 (comment)
Instead of allowing users to save files in a custom directory, this adds better handling of our temp files. It allows us to track all our recordings, so the user can remove them if they want.
Also it tracks an active recording, so if Kap crashes, we can recover it (hopefully playable or fixable) and direct the user to the file. Also allows for plugins to clean up after an unexpected crash.
Fixes #871 (or the need for it anyway)
Fixes #762