From 19252af34571e35b75d07b651d5f22d589c30ef4 Mon Sep 17 00:00:00 2001 From: Laurent Cozic Date: Wed, 21 Nov 2018 19:50:50 +0000 Subject: [PATCH] Desktop: Fixes #996: Allow editing multiple notes in external editor --- ElectronClient/app/app.js | 5 ++- ElectronClient/app/gui/NoteList.jsx | 23 ++++++++++- ElectronClient/app/gui/NoteText.jsx | 39 +++---------------- .../lib/services/ExternalEditWatcher.js | 36 ++++++++++------- .../lib/services/ResourceService.js | 15 +++++-- 5 files changed, 65 insertions(+), 53 deletions(-) diff --git a/ElectronClient/app/app.js b/ElectronClient/app/app.js index d6dedb74e1f..7833721dd4d 100644 --- a/ElectronClient/app/app.js +++ b/ElectronClient/app/app.js @@ -24,7 +24,7 @@ const InteropService = require('lib/services/InteropService'); const InteropServiceHelper = require('./InteropServiceHelper.js'); const ResourceService = require('lib/services/ResourceService'); const ClipperServer = require('lib/ClipperServer'); - +const ExternalEditWatcher = require('lib/services/ExternalEditWatcher'); const { bridge } = require('electron').remote.require('./bridge'); const Menu = bridge().Menu; const MenuItem = bridge().MenuItem; @@ -802,6 +802,9 @@ class Application extends BaseApplication { if (Setting.value('clipperServer.autoStart')) { ClipperServer.instance().start(); } + + ExternalEditWatcher.instance().setLogger(reg.logger()); + ExternalEditWatcher.instance().dispatch = this.store().dispatch; } } diff --git a/ElectronClient/app/gui/NoteList.jsx b/ElectronClient/app/gui/NoteList.jsx index 9c23ed40d32..5cd720bf5ae 100644 --- a/ElectronClient/app/gui/NoteList.jsx +++ b/ElectronClient/app/gui/NoteList.jsx @@ -16,6 +16,12 @@ const Mark = require('mark.js/dist/mark.min.js'); class NoteListComponent extends React.Component { + constructor() { + super(); + + this.itemRenderer = this.itemRenderer.bind(this); + } + style() { const theme = themeStyle(this.props.theme); @@ -169,7 +175,10 @@ class NoteListComponent extends React.Component { menu.popup(bridge().window()); } - itemRenderer(item, theme, width) { + itemRenderer(item) { + const theme = themeStyle(this.props.theme); + const width = this.props.style.width; + const onTitleClick = async (event, item) => { if (event.ctrlKey) { event.preventDefault(); @@ -269,6 +278,14 @@ class NoteListComponent extends React.Component { titleComp = {displayTitle} } + const watchedIconStyle = { + paddingRight: 4, + color: theme.color, + }; + const watchedIcon = this.props.watchedNoteFiles.indexOf(item.id) < 0 ? null : ( + + ); + // Need to include "todo_completed" in key so that checkbox is updated when // item is changed via sync. return
@@ -283,6 +300,7 @@ class NoteListComponent extends React.Component { onDragStart={(event) => onDragStart(event) } data-id={item.id} > + {watchedIcon} {titleComp}
@@ -313,7 +331,7 @@ class NoteListComponent extends React.Component { style={style} className={"note-list"} items={notes} - itemRenderer={ (item) => { return this.itemRenderer(item, theme, style.width) } } + itemRenderer={this.itemRenderer} > ); } @@ -329,6 +347,7 @@ const mapStateToProps = (state) => { notesParentType: state.notesParentType, searches: state.searches, selectedSearchId: state.selectedSearchId, + watchedNoteFiles: state.watchedNoteFiles, }; }; diff --git a/ElectronClient/app/gui/NoteText.jsx b/ElectronClient/app/gui/NoteText.jsx index bd9b202d26a..5f16ec530e9 100644 --- a/ElectronClient/app/gui/NoteText.jsx +++ b/ElectronClient/app/gui/NoteText.jsx @@ -305,6 +305,7 @@ class NoteTextComponent extends React.Component { eventManager.on('todoToggle', this.onTodoToggle_); ResourceFetcher.instance().on('downloadComplete', this.resourceFetcher_downloadComplete); + ExternalEditWatcher.instance().on('noteChange', this.externalEditWatcher_noteChange); } componentWillUnmount() { @@ -318,8 +319,7 @@ class NoteTextComponent extends React.Component { eventManager.removeListener('todoToggle', this.onTodoToggle_); ResourceFetcher.instance().off('downloadComplete', this.resourceFetcher_downloadComplete); - - this.destroyExternalEditWatcher(); + ExternalEditWatcher.instance().off('noteChange', this.externalEditWatcher_noteChange); } async saveIfNeeded(saveIfNewNote = false) { @@ -332,7 +332,7 @@ class NoteTextComponent extends React.Component { } await shared.saveNoteButton_press(this); - this.externalEditWatcherUpdateNoteFile(this.state.note); + ExternalEditWatcher.instance().updateNoteFile(this.state.note); } async saveOneProperty(name, value) { @@ -371,7 +371,6 @@ class NoteTextComponent extends React.Component { if (props.newNote) { note = Object.assign({}, props.newNote); this.lastLoadedNoteId_ = null; - this.externalEditWatcherStopWatchingAll(); } else { noteId = props.noteId; loadingNewNote = stateNoteId !== noteId; @@ -398,8 +397,6 @@ class NoteTextComponent extends React.Component { // Scroll back to top when loading new note if (loadingNewNote) { - this.externalEditWatcherStopWatchingAll(); - this.editorMaxScrollTop_ = 0; // HACK: To go around a bug in Ace editor, we first set the scroll position to 1 @@ -962,42 +959,16 @@ class NoteTextComponent extends React.Component { return splitStyle.join(marker); } - externalEditWatcher() { - if (!this.externalEditWatcher_) { - this.externalEditWatcher_ = new ExternalEditWatcher((action) => { return this.props.dispatch(action) }); - this.externalEditWatcher_.setLogger(reg.logger()); - this.externalEditWatcher_.on('noteChange', this.externalEditWatcher_noteChange); - } - - return this.externalEditWatcher_; - } - - externalEditWatcherUpdateNoteFile(note) { - if (this.externalEditWatcher_) this.externalEditWatcher().updateNoteFile(note); - } - - externalEditWatcherStopWatchingAll() { - if (this.externalEditWatcher_) this.externalEditWatcher().stopWatchingAll(); - } - - destroyExternalEditWatcher() { - if (!this.externalEditWatcher_) return; - - this.externalEditWatcher_.off('noteChange', this.externalEditWatcher_noteChange); - this.externalEditWatcher_.stopWatchingAll(); - this.externalEditWatcher_ = null; - } - async commandStartExternalEditing() { try { - await this.externalEditWatcher().openAndWatch(this.state.note); + await ExternalEditWatcher.instance().openAndWatch(this.state.note); } catch (error) { bridge().showErrorMessageBox(_('Error opening note in editor: %s', error.message)); } } async commandStopExternalEditing() { - this.externalEditWatcherStopWatchingAll(); + ExternalEditWatcher.instance().stopWatching(this.state.note.id); } async commandSetTags() { diff --git a/ReactNativeClient/lib/services/ExternalEditWatcher.js b/ReactNativeClient/lib/services/ExternalEditWatcher.js index b726be3fa18..95cc83e5c38 100644 --- a/ReactNativeClient/lib/services/ExternalEditWatcher.js +++ b/ReactNativeClient/lib/services/ExternalEditWatcher.js @@ -9,14 +9,20 @@ const spawn = require('child_process').spawn; class ExternalEditWatcher { - constructor(dispatch = null) { + constructor() { this.logger_ = new Logger(); - this.dispatch_ = dispatch ? dispatch : (action) => {}; + this.dispatch = (action) => {}; this.watcher_ = null; this.eventEmitter_ = new EventEmitter(); this.skipNextChangeEvent_ = {}; } + static instance() { + if (this.instance_) return this.instance_; + this.instance_ = new ExternalEditWatcher(); + return this.instance_; + } + on(eventName, callback) { return this.eventEmitter_.on(eventName, callback); } @@ -33,10 +39,6 @@ class ExternalEditWatcher { return this.logger_; } - dispatch(action) { - this.dispatch_(action); - } - watch(fileToWatch) { if (!this.watcher_) { this.watcher_ = chokidar.watch(fileToWatch); @@ -56,9 +58,17 @@ class ExternalEditWatcher { if (!this.skipNextChangeEvent_[id]) { const note = await Note.load(id); + + if (!note) { + this.logger().warn('Watched note has been deleted: ' + id); + this.stopWatching(id); + return; + } + const noteContent = await shim.fsDriver().readFile(path, 'utf-8'); const updatedNote = await Note.unserializeForEdit(noteContent); updatedNote.id = id; + updatedNote.parent_id = note.parent_id; await Note.save(updatedNote); this.eventEmitter_.emit('noteChange', { id: updatedNote.id }); } @@ -82,8 +92,8 @@ class ExternalEditWatcher { return this.instance_; } - noteFilePath(note) { - return Setting.value('tempDir') + '/' + note.id + '.md'; + noteFilePath(noteId) { + return Setting.value('tempDir') + '/' + noteId + '.md'; } watchedFiles() { @@ -181,15 +191,15 @@ class ExternalEditWatcher { this.logger().info('ExternalEditWatcher: Started watching ' + filePath); } - async stopWatching(note) { - if (!note || !note.id) return; + async stopWatching(noteId) { + if (!noteId) return; - const filePath = this.noteFilePath(note); + const filePath = this.noteFilePath(noteId); if (this.watcher_) this.watcher_.unwatch(filePath); await shim.fsDriver().remove(filePath); this.dispatch({ type: 'NOTE_FILE_WATCHER_REMOVE', - id: note.id, + id: noteId, }); this.logger().info('ExternalEditWatcher: Stopped watching ' + filePath); } @@ -231,7 +241,7 @@ class ExternalEditWatcher { return; } - const filePath = this.noteFilePath(note); + const filePath = this.noteFilePath(note.id); const noteContent = await Note.serializeForEdit(note); await shim.fsDriver().writeFile(filePath, noteContent, 'utf-8'); return filePath; diff --git a/ReactNativeClient/lib/services/ResourceService.js b/ReactNativeClient/lib/services/ResourceService.js index d0fd529f6a1..4598d1ef82a 100644 --- a/ReactNativeClient/lib/services/ResourceService.js +++ b/ReactNativeClient/lib/services/ResourceService.js @@ -36,7 +36,12 @@ class ResourceService extends BaseService { for (let i = 0; i < notes.length; i++) { if (notes[i].id === noteId) return notes[i]; } - throw new Error('Invalid note ID: ' + noteId); + // The note may have been deleted since the change was recorded. For example in this case: + // - Note created (Some Change object is recorded) + // - Note is deleted + // - ResourceService indexer runs. + // In that case, there will be a change for the note, but the note will be gone. + return null; } for (let i = 0; i < changes.length; i++) { @@ -44,8 +49,12 @@ class ResourceService extends BaseService { if (change.type === ItemChange.TYPE_CREATE || change.type === ItemChange.TYPE_UPDATE) { const note = noteById(change.item_id); - const resourceIds = await Note.linkedResourceIds(note.body); - await NoteResource.setAssociatedResources(note.id, resourceIds); + if (note) { + const resourceIds = await Note.linkedResourceIds(note.body); + await NoteResource.setAssociatedResources(note.id, resourceIds); + } else { + this.logger().warn('ResourceService::indexNoteResources: A change was recorded for a note that has been deleted: ' + change.item_id); + } } else if (change.type === ItemChange.TYPE_DELETE) { await NoteResource.remove(change.item_id); } else {