Sqlite database now gets the session_timestamp from the history session id

[dasbard]

(Sub pull request required for a nushell pull request)

Obtains the session_timestamp for the SqliteBackedHistory from the session (HistorySessionId) to avoid desynchronization.

The commented out update_session function on the History trait will be explained in the other nushell pull request.

[fdncred]

CI is mad but should be an easy fix with rustfmt.

When I originally wrote the sqlite sessions stuff, I included the ability to toggle the session. I'm wondering if that would be useful for what you're trying to do here. This is the original PR #562. Look for toggle history_session. I realize it's a bit different but seeing your commented code for update_session made me think about it.

[dasbard]

I have taken a look at toggle history_session but I have no idea how to properly do something similar inside the nushell evaluate_repl function (most likely) however a similar function to update the history session has been created.

I did run the cargo fmt and clippy commands but for some reason a lot of changes happened and I don't know why. Same with the one commit (nushell/nushell@2dcb821) in nushell.

[fdncred]

I did run the cargo fmt and clippy commands but for some reason a lot of changes happened and I don't know why. Same with the one commit (nushell/nushell@2dcb821) in nushell.

We probably won't be able to accept this the way it is, marking this draft for now. My guess is you have a rustfmt.toml somewhere that is influencing how it runs.

[codecov]

Codecov Report

Merging #674 (c089077) into main (93af55c) will not change coverage.
Report is 1 commits behind head on main.
The diff coverage is 100.00%.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #674   +/-   ##
=======================================
  Coverage   49.19%   49.19%           
=======================================
  Files          46       46           
  Lines        7930     7930           
=======================================
  Hits         3901     3901           
  Misses       4029     4029           

Files	Coverage Δ
src/edit_mode/vi/command.rs	30.48% <100.00%> (ø)
src/history/base.rs	83.95% <100.00%> (ø)
src/history/item.rs	44.89% <ø> (ø)
src/history/sqlite_backed.rs	75.08% <100.00%> (+0.32%)	⬆️

... and 3 files with indirect coverage changes

[dasbard]

My guess is you have a rustfmt.toml somewhere that is influencing how it runs.

I've been running the command suggested on the Contributing part of the reedline repository' readme in the same directory where I cloned the reedline repository. I don't know why it acted out like this. If you have any idea how I could troubleshoot this, that would be great.

[fdncred]

@dasbard My only guess is that you have one of these two things going on.

1. You have rustfmt.toml somewhere. read up on it here https://rust-lang.github.io/rustfmt/
2. You have some other formatter running in your editor that is changing the formatting.

I haven't seen a user have this problem before so I'm not filled with suggestions. Hope you can figure it out.

[dasbard]

@dasbard My only guess is that you have one of these two things going on.
You have rustfmt.toml somewhere. read up on it here https://rust-lang.github.io/rustfmt/
You have some other formatter running in your editor that is changing the formatting.
I haven't seen a user have this problem before so I'm not filled with suggestions. Hope you can figure it out.

Turns out I had a rustfmt.toml file inside my home directory for 4 months! I have looked into my~/ .cargo folder before and couldn't find anything and somehow missed that rustfmt.toml could also just be inside ~/

Maybe this could be avoided in the future with an empty rustfmt.toml file inside the repo overwriting user specific format files?

[dasbard]

Things should finally be fixed. I've also removed the update_session function for the History trait because that seems to not get anywhere and currently has no known usecase as far as I can think.

[fdncred]

Are there any tests you can write that run in our CI to ensure this feature doesn't get broken in the future? Also there are conflicts.

[dasbard]

I'm unsure about some tests. Maybe I should take a look at it.
About that conflict: I clicked on it but I can't seem to select anything to resolve it. I have no idea how to fix it.

[fdncred]

About that conflict: I clicked on it but I can't seem to select anything to resolve it. I have no idea how to fix it.

it looks like you need to pull the latest main branch and then git merge main in this branch and resolve the conflicts there, then push to the pr again.

[fdncred]

It's been a while and I'm lost as to what problem this PR is fixing. Can you talk more about that and fix the conflicts @dasbard?

[fdncred]

unresponsive