Skip to content
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

Create new user-friendly view #60

Merged
merged 14 commits into from
Nov 3, 2023
Merged

Conversation

lmcmicu
Copy link
Collaborator

@lmcmicu lmcmicu commented Sep 18, 2023

Potentially closes #58 (if we decide on this solution). Note that this branch has been forked from the undo-redo branch (which is the reason why the diff shows changes other than the one in b4484ff (which adds the new view)).

Before merging this PR, PR #59 must be merged to main first and then this branch should be rebased onto main. I am leaving this PR in draft mode to prevent inadvertently merging it before this has been done.

@lmcmicu
Copy link
Collaborator Author

lmcmicu commented Oct 28, 2023

This is ready for review as far as I'm concerned, but I am leaving the PR in draft mode since #59 must be merged first (see the description of this PR).

@jamesaoverton
Copy link
Member

This has been working for me so far. I would like two changes, please:

  1. Instead of *_user_view I would like to call this *_text_view.
  2. I'm finding the names get_last_undo() and get_last_change() confusing. I'd prefer something like get_undo() and get_redo().

@lmcmicu
Copy link
Collaborator Author

lmcmicu commented Nov 3, 2023

Done. I've changed user_view to text_view. I've also changed get_last_undo() to get_record_to_redo() and get_last_change() to get_record_to_undo()

…inserting new rows, even when there are errors.
This is not necessary in most cases and makes the code confusing.
There was a need, previously, because we used to call the
update/insert/delete single row functions directly on the conflict
table in some cases - the reason being that the update_row function
had two possible paths, one direct and one indirect. Now that it is
always indirect we can clean this up.
The only time where it is still needed is when we are actually
generating insert statements.
…rd_to_redo() and get_last_change() to get_record_to_undo()
@lmcmicu lmcmicu force-pushed the new-view-forked-from-undo-redo branch from 93aa011 to 8e17891 Compare November 3, 2023 15:24
@lmcmicu lmcmicu marked this pull request as ready for review November 3, 2023 15:30
@lmcmicu lmcmicu merged commit f4d1528 into main Nov 3, 2023
@lmcmicu lmcmicu deleted the new-view-forked-from-undo-redo branch November 3, 2023 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Be smarter about the distinction between conflict and normal rows
2 participants