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

Interaction between erase_prev and history.shared=false #45

Closed
chrisant996 opened this issue Dec 30, 2020 · 4 comments
Closed

Interaction between erase_prev and history.shared=false #45

chrisant996 opened this issue Dec 30, 2020 · 4 comments
Labels
bug Something isn't working

Comments

@chrisant996
Copy link
Owner

chrisant996 commented Dec 30, 2020

When history.dupe_mode = "erase_prev" and history.shared is false:

History entries can temporarily disappear in some instances.

REPRO:

  • Have history that includes echo hello.
  • Start two Clink instances.
  • In instance 1,
    • press Up until the echo hello history entry is shown.
    • press Enter.
  • Now the master history has echo hello removed, and echo hello was added to instance 1's session history.
  • In instance 2,
    • press Enter.
    • press Up repeatedly.

EXPECTED RESULT: echo hello will shown up in the available history.
ACTUAL RESULT: it won't be present again until after instance 1 exits and its session histories are appended to the master history.

@chrisant996 chrisant996 added the bug Something isn't working label Dec 30, 2020
@chrisant996
Copy link
Owner Author

chrisant996 commented Dec 30, 2020

This is an issue with the Clink 1.0.0 rewrite (or maybe it's always been present in Clink).

Maybe non-shared history mode can keep track of a high water mark that it's read, and never reload stuff from before that point. But history would still have to load the master history.

This might be very problematic to solve.

chrisant996 added a commit that referenced this issue Dec 30, 2020
Make `history.shared` default to true.
chrisant996 added a commit that referenced this issue Dec 30, 2020
Live with #45 for now; changing the default is disruptive, and there's
no clear solution yet.
@chrisant996
Copy link
Owner Author

chrisant996 commented Jan 19, 2021

The only way I can see to reliably solve this is to expand the history file format to accommodate versioning. Then erase_prev could mark entries as deleted by a future version, deletion would only be respected if the master history file version is greater than or equal to the deletion version, and the master history file version would not be updated until the session history is reaped from the session that deleted the history entry.

But that would open two other issues:

  • Must analyze how to extend the history file format in a reasonably efficient way.
  • Must come up with a version ID format that is both distributed and also can evaluate sequence. Because multiple sessions may be active, and a new session may start, and they all need to be able to accurately coordinate which ones know about which deletions.

UPDATE: a more functional idea is in the next comment.

What a mess.

@chrisant996
Copy link
Owner Author

chrisant996 commented Jan 19, 2021

For now I'll document this as a known limitation. 😞

The most feasible idea I've come up with so far is for each session to keep separate logs of pending deletions and pending additions. Additions can be reaped on session end, but deletions can only be reaped when all sessions have ended. (The history command would apply a session's pending additions and deletions only in memory, so that the resulting history list matches what the session itself sees.)

I think that would fully addresses the problem, but some further mental gymnastics are needed to verify the hypothesis.

I'm on the fence about whether that's a reasonable level of complexity for a solution.
"Simple can be harder than complex" -- Steve Jobs.

@chrisant996
Copy link
Owner Author

Fixed by keeping track of removals separately, applying them in memory for a given session during history, and applying them back to the master history when a session ends. Added tests as well.

Unfortunately the git fast-forward ruined the history. I'm still letting my opinion of fast-forward form -- I don't yet understand why so many people seem to think that fast-forward merges of topic branches gives a "more accurate" view of history. To me it's a very wrong view of history because it makes it appear as though part of a topic branch feature was done in the topic branch and part of it was done in master. When in reality the entire feature was done in the topic branch. I see claims about various "problems" and "dragons" if you don't fast-forward, but so far I haven't seen any actual evidence of problems, only an apparent preference for throwing out parallel history and rewriting it as interleaved mixed up synthesized sequential history that never happened. 🤷‍♂️

Anyway, the final commit from the fix_erase_prev topic branch was cc44629.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant