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

feat/rgb-color #437

Merged
merged 3 commits into from
Nov 12, 2024
Merged

feat/rgb-color #437

merged 3 commits into from
Nov 12, 2024

Conversation

k86td
Copy link
Contributor

@k86td k86td commented Nov 6, 2024

ref #435

Copy link
Owner

@zhiburt zhiburt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me @k86td

Sorry that I did not respond faster.

Take care.
Have a great week.

Comment on lines +306 to +309
[[example]]
name = "interactive"
path = "example/interactive.rs"
required-features = ["derive"]
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking you're new in rust,
you had figured it pretty fast 😄

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's been misspelled "examples/interactive.rs"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah crap, since you've already merged it, do I commit on my branch and create a new pull request with the changes?

I was thinking you're new in rust

Lol, this is the first library I'm actually contributing to, thanks for the help and patience:). The things you wrote was actually my reference.


const CLEAR: &str = "\u{1b}[2J";

type Step = Box<dyn Fn(&mut Table) -> (u64, &mut Table)>;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh kind of hard to understand right?)

Probably will need to refactor it afterall

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. I could put the steps in its own struct to try and make it a little bit cleaner if that's what you meant.

Like:

struct StepResult {
  // could have `Duration` directly here instead of `u64` milliseconds
  duration: u64,
  table: &mut Table,
}

struct Step {
  action: Box<dyn Fn(&mut Table) -> StepResult>
}

Not sure if this is syntactically correct, I haven't tried it.

@zhiburt
Copy link
Owner

zhiburt commented Nov 12, 2024

And thanks for example

@zhiburt zhiburt merged commit 680196f into zhiburt:master Nov 12, 2024
50 of 75 checks passed
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