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

Make history-related items (de-)serializable #678

Merged
merged 5 commits into from
Dec 6, 2023

Conversation

ClementNerma
Copy link
Contributor

@ClementNerma ClementNerma commented Dec 6, 2023

This PR makes history-related items like HistoryItem and HistoryItemId (among others) serializable and deserializable through serde. This allows building custom History more easily.

@fdncred
Copy link
Collaborator

fdncred commented Dec 6, 2023

I generally don't have problems with your recent PRs but I'd like to see something in the description that we can refer to later, even though these are quite simple changes. Obviously the CI needs to be green too. :)

Where are you integrating reedline that is forcing you to make these changes? I'm just interested to see what you're doing.

@ClementNerma
Copy link
Contributor Author

I generally don't have problems with your recent PRs but I'd like to see something in the description that we can refer to later, even though these are quite simple changes. Obviously the CI needs to be green too. :)

Just edited the description and updated the code so the CI should pass.

Where are you integrating reedline that is forcing you to make these changes? I'm just interested to see what you're doing.

I'm building full-blown custom shell, and I'm currently polishing the last parts, like the history. I want to store specific informations, maybe perform history synchronization between shell instances etc. So I absolutely need a custom History for that.

@fdncred
Copy link
Collaborator

fdncred commented Dec 6, 2023

thanks for the info. i landed your pr that had a green ci.

@ClementNerma
Copy link
Contributor Author

CI is now green.

Copy link

codecov bot commented Dec 6, 2023

Codecov Report

Merging #678 (9079f6c) into main (ca2f6c8) will not change coverage.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #678   +/-   ##
=======================================
  Coverage   49.16%   49.16%           
=======================================
  Files          46       46           
  Lines        7926     7926           
=======================================
  Hits         3897     3897           
  Misses       4029     4029           
Files Coverage Δ
src/history/item.rs 44.89% <100.00%> (ø)

@fdncred
Copy link
Collaborator

fdncred commented Dec 6, 2023

one of the other prs caused a conflict here

@ClementNerma
Copy link
Contributor Author

Fixed.

@fdncred fdncred merged commit 43944ee into nushell:main Dec 6, 2023
8 checks passed
@fdncred
Copy link
Collaborator

fdncred commented Dec 6, 2023

good luck, have fun!

@fdncred
Copy link
Collaborator

fdncred commented Dec 6, 2023

I'm building full-blown custom shell, and I'm currently polishing the last parts, like the history. I want to store specific informations, maybe perform history synchronization between shell instances etc. So I absolutely need a custom History for that.

Is this public somewhere that i can look and play with it?

@ClementNerma
Copy link
Contributor Author

Sure: https://github.com/ClementNerma/ReShell

Please note that there is no tutorial or documentation yet, so you'll have to decode the parser and AST files yourself currently :p

Here is a little example (note that they are still some bugs, especially on the stack):

fn fibonacci(value: int) -> int {
    return if $value < 1 {
        0
    } else if $value <= 2 {
        1
    } else {
        fibonacci($value - 1) + fibonacci($value - 2)
    }
}

for i in range(1, 100) {
    echo "fib $i = ${fibonacci($i)}"
}

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.

2 participants