In this case, I want to keep both the rename regex -> regexp, and the change from /.../ to %r{...} regexes. If we have an editor, then this becomes tractable.
Designs
Child items
...
Show closed items
Linked items
0
Link issues together to show that they're related or that one is blocking others.
Learn more.
I think we can do file-by-file resolution by storing the resolved file blobs in local refs in the git repository, in the same way that we have the keep-around and merge request refs. This could be per-user, or not, and there's nothing magical about those refs, so you could also push to them, but I think it could work.
Technically we could probably store the blob IDs anywhere, as they won't be GCed for two weeks, which should be long enough to resolve conflicts. We should also remove these refs when they are no longer valid, which is tricky if we don't know if they were user-supplied or not. I think the repository is the place for these, though.
We'd also need to check if the stored file has any conflicts with the current target HEAD, and invalidate it if the source HEAD updates in the mean time.
@DouweM and I just chatted about this, and here are the conclusions we came to:
If nothing major goes wrong with !5479 (merged), and we really believe, we might be able to get this in 8.11 too!
Merge conflicts editing should operate on the whole file only. Operating on individual conflicts is a) fiddly to implement from the backend and b) going to have edge cases where, say, you end up with a syntax error because you need to edit a line outside the conflict region.
Instead of doing this step-by-step (which is what I was thinking about above) we could do this all in one POST request from the frontend. How?
Show the view as it is now, but with an 'Edit this file' button for each file. (If a file can't be shown as it is now, we can just say that it's only editable manually.)
If you've already clicked to pick ours / theirs on the conflict sections, then edit the file, we resolve those conflicts before showing you the file.
Once you're done with the file, you click a button and return to the main view. The file does not show conflicts any more, but maybe an info section saying it's been manually resolved.
All of these edit conflict resolutions happen on the same page in the browser, and are kept in the Vue model. Once all conflicts are resolved, we send them all at once to the backend.
From the backend perspective, we can provide an API to output a file by path with conflict markers, which takes some optional section resolutions. The existing resolve conflicts API will be extended to accept full file contents as well as section IDs.
From the frontend and UX perspective, this is probably a ton of work, but let's talk about it!
I'd really love this functionality, the MC UI MVP is awesome, but unfortunately I can't use it very often since I usually require a combination of two changes.
@smcgivern I am looking into the UX for this and I am wondering if it is a good or bad idea to make the lines editable from the same view after selecting "theirs" or "ours"? At first I was thinking exactly what you have listed out above but was curious if editing on the same view was a possibility? Maybe this is a bad idea or not even possible, I'm not sure.
I think that's a good UX solution, but I think the editor is a different component from static diffs. Maybe we can have an "Edit this" button that embeds an editor inside the diff?
@tauriedavis that's an interesting question! The big constraint from the backend side (as in, turns this from straightforward-with-issues to total-nightmare) is that there is one single HTTP request containing all resolutions at once. As we're using Vue, it doesn't necessarily need to be the same 'view' for everything, but we do need one single resolution from the frontend.
Beyond that, I'm pretty flexible, as I think my own personal thoughts on this might be too coloured by the way resolving conflicts works on the command line. I'm happy to talk about this when you're around today too, if that helps.
At the moment, we return an object with some metadata and a files array. Those files each have sections, as well as old_path, new_path, etc.
A file will now have a type which is either text or text-editor. (In future, we can add image / binary for picking ours / theirs for images.)
If a file can't be parsed to show the sections, it will be text-editor. This means it can only be resolved in the editor.
Otherwise, if it's a file with sections, it will be text.
In both of those cases, a file will also have a content_path to a URL containing the full file contents.
This path could return JSON, but it seems like text/plain would also work. If we want JSON, then I suggest it returns basically the same metadata as a regular file (old_path, new_path, blob_icon, etc.), along with a content string, and without any sections.
For resolving:
We currently pass an object with sections and commit_message keys.
This will be changed to accept a commit_message and a files key.
files is an array of objects with new_path, old_path, and eithersections or content keys, depending on how it was resolved.
Those two keys match the way those keys work for reading.
If a file has incomplete sections, we fail as before.
If a file's passed content matches the file content with conflict markers from Rugged::Index#merge_file, we fail to resolve it. (We can't really verify what changes were made, but we can verify that changes were made.)
I'm working on JSON Schemas for these to make this clearer
@tauriedavis I've been seeing how this feature is working right now and I think what @smcgivern proposes seems pretty doable. I have only one suggest. Editors should be inline inside each file so we can edit multiple files at once. And probably we should consider how errors are presented if something fails.
@alfredo1 Sorry, which proposal seems pretty doable? I'm not totally clear on the direction you guys think is best. And what view are you suggesting to have the inline file editor?
@alfredo1 Makes sense. My proposal was that after the user selects one of the options (theirs or ours), the file would then be an inline editor so they could edit the whole file. Maybe this is too hard?
@tauriedavis I think we should allow only one way to resolve conflicts per file. manually or selecting theirs/ours. Mixing the two will make it difficult for both the backend and frontend. Unless I am missing something.
Okay, so it appears a toggle between editing inline and (what I have named) interactive mode (let me know if you have a better name for it) should work. It would be nice to keep the background colors when in inline mode. Thoughts @cperessini?
@tauriedavis I think that looks nice! There's a problem with using a toggle, though: you can't go from the editor mode to interactive mode unless you've made no changes. Even then, I think we might want to forbid that so that this is always a one-way operation.
There are two types of file we can resolve at the moment. I've added some questions too
File available for interactive mode.
In interactive mode, the user needs to pick ours / theirs for each conflict section.
If they choose to switch to inline editor mode, we discard the existing ours / theirs choices. (Still an open question, maybe?)
Once they are in the inline editor mode, they can't go back - they must resolve the conflicts in the editor.
This mode shows the entire file contents in an editor.
Question: should there be a 'done editing' button here, to hide the editor? What would it show instead?
Question: what max height should the editor have? The file can be as big as the GitLab CE CHANGELOG, which is pretty big.
File only available for inline editor mode. (This is a pretty rare case at the moment.)
We can either show the editor always, or a message to open the editor.
The user must change the file content in some way to resolve the conflict.
@tauriedavis@smcgivern I think we shouldn't disallow going back to interactive mode because users may want to try out the manual editing feature to see how it works, or start using it and then realize ours/theirs is enough.
Save your progress
I included a Save changes button so users can tell the frontend that they're done making changes.
Done editing
When the user presses the save button the progress bar turns green to show that the conflicts for that file have been resolved, and the save button becomes disabled. If the user makes any more changes, the button becomes clickable again.
Going back to interactive mode
If the user toggles back to interactive mode or clicks the Cancel button, they are prompted to confirm that they want to discard their changes. I don't really like this implementation because it changes the content of the Save/Cancel bar and is too far away from the toggle in the header, which could lead to users missing it. I faded the editor to make it more noticeable, but I still think it isn't optimal.
Interactive mode unavailable
In that case, the editor is showed by default and the interactive mode side of the toggle is disabled. I'm not sure we should have a Cancel button in this one, because it can't really do anything.
@tauriedavis nice! With @cperessini's mockups I think we are close to the whole flow of solving a conflict on the frontend.
Regarding the discard button not being always visible. maybe we can prompt a dialog like Facebook does when you want to go to another page and didn't finished to publish a note. so this way the user will not miss the warning dialog.
OK, that's why I don't do UX @cperessini that makes perfect sense to me!
I think @alfredo1's modal proposal also makes sense because otherwise, the toggles for inline edit / interactive mode look like they're still clickable when you need to make the discard / cancel decision.
@alfredo1@smcgivern Great idea. I didn't consider it because I believe we use browser dialog boxes instead of custom modals, which would be pretty bad to use here. But it'd be really good to create a new element for our UI that we can use in places like this.
A note about my design. If the user has hit the Save button, but then they start editing again, I think the progress bar should also turn back to grey. Otherwise they could hit the Commit conflict resolution button and we couldn't know if they want to commit the current content or what they had previously saved.
@cperessini that makes sense to me, so a user can only resolve the conflicts if they've selected ours / theirs for every section in interactive mode, and saved every file (without changing it since) in inline editor mode.
Awesome! Thanks @cperessini for those flows Super helpful
I wonder if the save button is necessary? It feels like an extra unnecessary step for the user. Is it possible to just make the progress bar green when the file has been edited and if they try to toggle back to interactive mode, then we display the discard changes prompt? Any downsides to this? @cperessini
We do have some custom models currently. It would look like this:
Another option is to have the message pop down under the toggle which is less intrusive than the modal:
@tauriedavis If we don't get explicit confirmation then changing a single character would turn the bar green and let the user commit the resolution. I'm not sure that's a bad thing, since it's not too different from clicking Use ours by accident for example. But even then having the user tell the app they're done does feel more robust. @smcgivern what made you think of the save button originally?
I like the second approach better because it's less obtrusive
@cperessini the only reason I was thinking of a save button originally is that I was imagining it as a modal operation: so you'd edit one file, mark that you were done with that one, then edit the next file. But there's no technical reason it has to be that way, and it looks like this can be clear even with multiple editor instances on the same page. So let's not treat that as a requirement
@tauriedavis I really like the version under the toggle
@alfredo1 please do, and let me know if there are any issues! Note the frontend will need to be changed to match the new format for passing resolutions so that each sections array is inside a file object with a new_path and an old_path for the existing feature to work. Ping me if you have any questions!
I think because they won't be able to submit the commit until they make a change (right?), we should keep the bar. I'd like to play with it to see if it is a weird interaction, though. @cperessini@alfredo1
It would be cool if the edit mode had a button to jump from one conflict to the next (probably just based on the <<<< line), so that the user doesn't accidentally miss a conflict, and commits a conflict.
I'm not sure about "Interactive mode", because in a sense both modes are interactive, and the difference between the modes is not just the interactivity, since the one (edit) is actually much more powerful than the other ("interactive"). Something like "Pick ours or theirs" sounds kind of lame, but it's really descriptive and totally accurate.
Maybe we should have a way to switch to Edit mode by each individual conflict as well, since if it's a long file with a lot of conflicts, the Nth conflict may be the first one that isn't resolvable ours/theirs, and it would be nice to have "Edit inline" besides "Use ours"
Maybe we should have a way to switch to Edit mode by each individual conflict as well, since if it's a long file with a lot of conflicts, the Nth conflict may be the first one that isn't resolvable ours/theirs, and it would be nice to have "Edit inline" besides "Use ours"
We don't currently preserve the conflict resolutions from the 'pick mode' when setting up the editor!
It would be cool if the edit mode had a button to jump from one conflict to the next (probably just based on the <<<< line), so that the user doesn't accidentally miss a conflict, and commits a conflict.
For that I think we need to locate the conflicts blocks in the editor. And also because of this:
We need to know how many conflicts the user have resolved when in Edit mode so I can show the correct amount of resolved conflicts.
@cperessini@smcgivern Not sure If I understand correctly. How the conflicts counter will be calculated? If a file is resolved with Edit mode, how it will affect the conflicts counter?
@alfredo1 There's not gonna be a save button. We're gonna go from this:
To this after the user edits:
There are a few alternatives on when to turn the bar green:
After the user changes a single character in the file.
When they click away from the editor if they have made changes (probably not good because some people will want to go from editing to clicking Commit)
After they are done editing. I don't know if it's possible to detect when users haven't typed in a while.
When they've made a change to every conflict in the file (at least one character per conflict). I think this is the most appropriate solution because that way we know they took care of everything, but I think it might not be possible to implement it.
Are we gonna be checking if users really solved the conflicts when they hit commit? Maybe by checking if there are any sections with <<<<<<<<< HEAD ... >>>>>>>>> origin left. If we did, we could have just one simple check for turning the bar green and determine later if it's a valid edit.
Are we gonna be checking if users really solved the conflicts when they hit commit?
A file is allowed to contain conflict sections! For example, if you wanted to commit an example of a merge conflict :-) That's why the back-end check is just that the contents changed somehow.
If we do this check, it should be done on the front-end and be purely advisory, not blocking the resolution.
Every update (adding or removing content) willl be saved.
After every update the file will be marked internally as resolved. So the Commit conflict resolution button can be enabled.
A resolved file will have the green bar.
If the user undoes every modification and saves an exact version of the original version of the file, it will be marked as unresolved and the Commit conflict resolution button can not be enabled.
The thing is that @smcgivern's comment has made me think that there will be times when users want to submit the resolution without making any changes. In that case we could just hide the progress bar for manual editing and make everything simpler. That means that just switching to manual mode would mark that file as resolved.
Either way works for me. Do any of you have a preference for one of the two?
Yeah, actually I was wondering the same that's why a Save changes button had more sense to me. Personally I prefer a Save changes button. if not we need a visual indicator that the file is saved or resolved since the start.