I had some additional thoughts about the conflict resolution UI:
Each conflict section is independent and can be resolved in any order. The state is saved after each one so partial progress is never lost.
Seeming as you removed the git style conflict markers, how would you intend to keep the remaining conflicts in sync with the content, without having to re-compare the whole note body after every change which is accepted? Maybe it would make sense to still make use of conflict markers, but only put them into the note created in the conflicts folder rather than the original note. But I think you should only create the merged content with conflict markers at the point of making the first change via the conflict UI. This is better firstly to avoid performance overhead on the sync, and secondly because of needing to retain backwards compatibility using 2 way sync.
There’s also an edge case that if you get a conflict on a note, don’t address the conflict and then get another conflict on the same note, the base version in the sync_item table may have been updated, meaning you have lost an eligible base version for the 3 way sync. In that case, you also need to use 2 way sync. To determine whether to use 3 way or 2 way sync, you’ll also need to add a column linking the base version to a particular conflict. You could add a base_conflict_note_id column to sync_items to do this, which is updated at the point of creating the conflicting note. Then in the conflicts UI, if that column matches the id of the note, then it determines that you can do 3 way sync.
Regarding the note title, as mentioned before, you should not insert conflict markers there due to the single line restriction. But as there could only ever be one line for it to apply in the conflicts UI, conflict markers are not needed to retain a partial resolution state anyway.
Also some useful considerations for the conflicts UI would be:
Having the ability to undo and redo changes made via the conflicts UI, in the same way as the normal markdown / rich text editor
Adding apply all left / apply all right buttons, which apply all remaining changes from the left or right side including conflicting lines. Or some variation of this. Regardless of how this is implemented, it would be good to still provide some way to at least be able to accept the contents directly from the conflicting note if the conflicts UI is going to be mandated when opening it
So sorry for replying late , i was having an exam; thank you for the additional thoughts, these are really helpful
On partial progress tracking : putting conflict markers into the conflict note body makes sense, writing them lazily only when the user makes their first change via the UI avoids any performance overhead during sync and keeps backward compatibility and the markers in the conflict note then serve as the persistent state so reopening the UI resumes exactly where the user left off and the original note stays completely clean
on the double conflict edge case: adding a base_conflict_note_id column to sync_items is the right approach , it gets set to the conflict note id at the point of conflict note creation and the UI checks whether this column matches the current conflict note before deciding to use three-way or two-way diff, if there has been a second conflict in the meantime and the base is stale, it falls back to two-way automatically, does this okay?
On title conflicts i will agree, no markers needed there since there is only one line, the UI just shows both versions and the user picks one, no partial state required
for undo/redo i will implement an in memory undo stack in the react component since the conflict note body already persists the resolution state through markers, the undo history only needs to survive while the UI is open which an inmemory stack handles finne
for apply all I will add "apply remaining: mine" and "apply remaining: theirs" buttons that only affect sections not yet resolved, skipping any the user has already handled individually and a confirmation prompt before applying to avoid accidental overwrites,
if these approaches are okay , i will update the proposal ; and suggest any changes if required
Yep all sounds good. One other thing I was thinking in the meantime:
In memory undo redo stack is fine, but if you auto navigate to the next conflict or the original note upon apply the final change to merge, if you accidentally merge it, then you would lose the undo history for that last change. What about before auto navigation happens, you have some kind of prompt which says All changes have been merged. Continue to next conflict? / Do you want to leave the conflict editor?
yeah that is a good point this auto navigating immediately would clear the undo history before the user has a chance to catch a mistake on the last section i will keep a confirmation prompt before navigation makes sense something like "all sections have been resolved. continue to next conflict?" with a stay here option. that way the user can still undo if needed before leaving the conflict editor, is this good to go?
I wanted to clarify one thing about the conflict resolution UI flow. In my current PoC I have implemented it slightly differently from what you described like instead of the user going to the conflicts folder and opening the conflict note there, a banner appears on the original note itself saying it has unresolved conflicts with a review conflicts button, clicking that opens the resolution panel on the right side of the editor this means the user never has to leave the note they are already on, I feel this will be a better user experience
would this approach be acceptable or do you feel it is better to keep it tied to opening the conflict note in the Conflicts folder?
It’s a decent idea, but I’m not sure how easy this ‘notification banner’ would be to implement on mobile. I think you can potentially do both, and ditch the banner idea if it is too hard, or just do the banner on desktop if too hard on mobile. It could also be a stretch goal.
So I’d say implement as per the original plan of opening the conflicts UI when the user opens a note in the conflicts folder, but optionally there could also be the ability to jump directly to the relevant conflicting note via a banner, which would just navigate to the conflicting note. The conflict UI would automatically open when navigating there anyway, so you don’t need to duplicate any logic to show the conflicts UI
Also I realise actually the idea of adding in the conflict markers as you accept changes will be added to the conflicting note, not the base note. And that is desirable, to avoid the merge markers showing in the normal editor view when outside of the conflicts notebook and to avoid finalising the result until you complete the merge.
It does mean though that while changes are persisted as you go, you do need a specific action to copy the final result to the original note, delete the conflict and update the base version. So the dialog which was mentioned after merging all changes is the at which you could apply this change (when you choose continue instead of stay). However if you choose to stay, there will need to be a button to Finish, in order to be able to apply the completion steps. You could make this Finish button always visible, but make it disabled until all changes have been addressed
Actually, if you have a Finish button, I guess you don’t really need the dialog at all. If the Finish button is disabled until all changes have been ackowledged, this will protect against accidentally completing the resolution in one press.
It does mean that the ‘Edit manually’ option needs to have a means to acknowledge when you have completed the change though. Also using the undo / redo actions should update the state of the Finish button and unresolved / reresolved changes
on the banner, i will implement the core flow as opening the conflict UI when a conflict note is opened in the conflicts folder and the optional banner on the original note will just navigate to that conflict note since the UI opens automatically anyway, so no logic is duplicated and If it turns out to be complex on mobile I will treat it as a stretch goal or keep it desktop only
on the conflict markers, agreed that they go into the conflict note only, not the original note, this keeps the original note clean until the user explicitly completes the merge and avoids markers showing up in the normal editor view outside the conflicts folder.
on the finish button and dialog, yeah that right that if the finish button is disabled until all sections are acknowledged then the confirmation dialog is not needed at all. I will remove the dialog entirely and just keep the Finish button.
for edit manually, i will add a done button inside the edit flow so the user can explicitly acknowledge when they have finished editing that section , this marks it as resolved and counts towards enabling the finish button.
on undo/redo, i will make sure that undoing a resolution marks that section as unresolved and disables the finish button again, and redoing it marks it back as resolved, so that the finish button state will always reflect the actual current resolution state accurately
I will update the proposal if all these approaches are okay
The only addition to the sync pipeline is writing base_body and base_title to sync_items after every clean upload in Synchronizer.ts
You didn't mention the base_conflict_note_id column here
When all sections are resolved and the user confirms, the merged result is written to the original note, the conflict note is deleted, and base_body in sync_items is updated explicitly. Sync is never blocked at any point.
Needs to include the base_title is updated too
On completion the merged result is written to the original note, the conflict note is deleted from the Conflicts folder, and base_body and base_conflict_note_id in sync_items are updated explicitly
Same here, and also in the diagram in the 'Changes to the Joplin codebase' section
The most important detail to get right is updating both base_body and base_conflict_note_id in sync_items when the user completes a merge
base_title is also missed here
The concurrent editing case where a new sync arrives while the user has a conflict note open will be confirmed with mentors during community bonding.
This is not essential to solve, but to be honest I think your banner idea solves this very well. So you can mention here that handling this is non-essential, but adding the banner on both desktop and mobile is desirable if possible within the timeline
Other comments:
You did not mention this anywhere in the updated proposal: "On title conflicts i will agree, no markers needed there since there is only one line, the UI just shows both versions and the user picks one, no partial state required". Please add details about this in the "Storing unresolved conflicts" section
I think it would be helpful to differentiate between additions + deletions vs actual conflicts in the conflict resolution UI. Additions + deletions can simply have 'Accept' and 'Reject' buttons which will accept from whichever side is changed. For conflicts, they should be made to stand out a bit more (eg. a red Conflict heading) and they will have the User Mine / Use Theirs / Edit Manually buttons. In addition to 'Apply remaining mine' and 'Apply remaining theirs' buttons, it would be helpful to also have an 'Apply non conflicting changes' button. All these changes would just need to be added to the "Conflict resolution UI for desktop" section, including updating the flow diagram in that section
One more thing I thought of. It would also be good to have a ‘Keep both’ button on the conflict resolution UI, which will be the same as ‘Keep remaining mine’, except it will copy the output to a new note in the same notebook as the original (instead of replacing the original note), but with a distinguishing suffix on the note title. This is necessary because the mandatory conflict resolution UI for notes in the conflicts folder would prevent you keeping both copies otherwise, which is currently possible in the existing conflicts resolution in Joplin. When clicking this new button, there should be a confirmation dialog, otherwise it would be a 1 press action which would lose the ability to undo, if you made a mistake resolving earlier changes
That makes sense and it’s right that it is necessary to preserve existing behaviour then i will add a “keep both” button that works like apply remaining mine but copies the output to a new note in the same notebook with a suffix on the title instead of replacing the original and a confirmation dialog will appear before it runs since it is a one press action that bypasses the undo stack, I added this to the proposal
Thanks, I have reviewed the changes. Some minor feedback:
All sections are shown to the user sections where only one side changed and sections where both sides changed. Each section shows both versions side by side with Use Mine, Use Theirs, and Edit Manually options.
I’d suggest removing this paragraph, as the description that follows already describes it in more detail, and this paragraph is written in a way which is a bit confusing when combined with the latter info.
Title conflicts are handled differently. Since the title is a single line there is no need for conflict markers. The UI simply shows both the local title and the remote title and the user picks one. There is no partial state to track for title conflicts since there is only ever one choice to make.
I think the resolution options for the title should align with a single change to the body. If there is a change on one side only then offer Accept or Reject options. If it is an actual conflict then offer Use Mine, Use Theirs or Edit Manually.
On a wider note, there’s some additional things which I think are not addressed by the solution:
There’s no definition of how the 2 way merge will work. Would the resolution UI be the same as 3 way merge except it assumes every difference is a conflict, because there is no base version?
There is no implementation detail about how the conflicts UI will be able to tell the difference between a conflict which has been untouched by the user, and a conflict which has all sections resolved (no conflict markers remain in the conflict note body). The same question applies for the title field too, which does not even use conflict markers. And for 2 way merge, there also needs to be a way to determine when each change is resolved, as I don’t think the conflict marker approach would work in that case. Also, when for 3 way merge, the conflict marker approach only stores the conflicting changes, not the additions and deletions, so it can’t be used to determine when non conflicting changes are resolved
When using edit manually, where will the user changes be stored? For 3 way merge, storing user input in the mine side of the conflict markers should be ok, but how would it work for 2 way merge and the title field?
Some thoughts for a solution solve all these issues:
For 3 way diff, instead of using conflict markers, add new fields conflict_base_body, conflict_base_title, conflict_remote_body, conflict_remote_title to the conflict note. Initially, these will be a copies of the base note and remote note values, but as a user applies resolution actions, the relevant sections are aligned to match on all 3 versions, so that the section will be removed when the 3 way diff is re-evaluated. When using the manual edit option, changes would be made in the conflict note body / title directly. This solution also means that resolution for the note body and title can be handled in the same way. In order to avoid excessive content duplication, these 4 fields only get populated upon making the first resolution action. Note that the undo and redo actions will need to apply the change go all 4 of these columns in addition to the conflicting note body and title. So it would be best that these undo an redo actions are dedicated buttons in the conflict ui which undo and redo 1 resolution step, rather than being tied to the editor. The editor should have a separate undo and redo state when using the manually edit option
For 2 way diff, the same logic applies as the above solution for 3 way diff, except only the conflict_remote_body and conflict_remote_title will be populated, and resolving sections will align just 2 versions instead of 3. The undo and redo should exclude conflict_base_body and conflict_base_title
When any of the new conflict_ columns are populated, this will be able to distinguish when conflict resolution has been started, vs when all conflicts are resolved
When any of the new conflict_ columns are populated, this will overide the logic to determine to use 3 way diff vs 2 way diff based on whetger the base_conflict_note_id matches. If only conflict_remote_body and conflict_remote title are populated, 2 way diff is used. If all 4 columns are populated, all 4 columns are used
In order to avoid excessive memory usage, the undo / redo stack for each resolution step should store diffs, rather than full note contents of all 2/3 versions of the note. So diffs should be evaluated at the point of applying a resolving action, just before updating the stored values. The conflict note content ideally should not be updated in the db until the done button is pressed, when using the edit manually option
Thank you for the detailed feedback , I have updated the proposal based on your suggestions
I removed the redundant paragraph at the top of the conflict resolution UI section. For title conflicts I updated the handling to match the body accept or reject when only one side changed, and Use Mine, Use Theirs or Edit Manually when both sides changed.
for the three unaddressed problems I have adopted your suggested solution completely, the conflict markers approach is replaced with four new columns on the conflict note - conflict_base_body, conflict_base_title, conflict_remote_body and conflict_remote_title. these are populated lazily on first resolution action. The presence of these columns indicates resolution is in progress and clearing them indicates completion. Two-way diff uses only the remote columns and treats every difference as a conflict. Edit Manually writes directly to the conflict note body or title with the DB updated only on Done. Undo and redo are dedicated buttons in the conflict UI storing diffs, separate from the editor undo/redo state.
I have updated the storing unresolved conflicts section and the implementation plan as per the changes made
Hi @CalebJohn, I have updated the proposal based on the discussion with @mrjo118 and would appreciate it if you could take a look on my proposal when you get the chance
I have updated the storing unresolved conflicts section and the implementation plan as per the changes made
@varun I have another point to add to this which I didn’t have time to write up yesterday.
When using the conflict_remote_body, conflict_remote_title, conflict_base_body and conflict_base_title fields, it is possible that when they are populated by a resolution in progress, the values can become stale when either the remote version (the note in the original notebook) is updated, or if there are multiple conflicts for the same note, and a different conflict for that note completes conflict resolution, while the other conflict resolution is still in progress. In both of those cases, the remote version is updated as a result, and if the original conflict is then resolved, those changes to the remote version will be lost.
In order to remedy this, a conflict_remote_updated_time column can be added, which is set to the updated_time set on the remote version at the time of the conflict being created. Whenever a note is opened in the conflicts folder, it will check if the conflict_remote_updated_time is the same as the updated_time on the original note in the notebook. If it matches, continue the logic as already described. If however the values do not match, all of the conflict_remote_body, conflict_remote_title, conflict_base_body and conflict_base_title fields will be cleared and the conflict_remote_updated_time is updated to the new value. This results in all existing partial conflict resolution for that conflict to be lost, but the conflict will reconsider the 3 way / 2 way diff using the latest version of the remote note. In order to avoid surprise to the user, this invalidation should only happen upon opening / switching note within the conflicts folder, rather than happening in real time if the remote note is updated. This creates a small possibility of data loss, but it is however very unlikely, as the yser would have to be editing the conflict at the same time as editing the remote note (possible with both notes open in separate windows on the desktop app). This invalidation logic offers a compromise, to avoid data loss if the user accidentally or intentionally attempts to edit multiple versions of the note while a conflict is partially resolved, so that they will instead lose their progress for the partial conflict resolution in this scenario