-
Notifications
You must be signed in to change notification settings - Fork 0
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
refactor: use a separate message display trait instead of println!() #69
Conversation
This reverts commit 79b146e.
litt/src/main.rs
Outdated
// everything that does not require litt index | ||
|
||
// Print existing litt indices | ||
if cli.list { | ||
println!("Currently available indices:"); | ||
cli.display(Message::Info("Currently available indices:")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should add another type for Message called "CLI-MSG" or something. Because these messages are not Logs (where "Info: Currently available indices" would be wanted), but the actual user output where the output should simply be ("Currently available indices")
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it. I thought it might be good to emit semantic messages – so that the message knows what content it actually contains and then the implementation to display it, decides how to display that content. What do you think about that? See latest commits for this :)
index/src/index.rs
Outdated
let relative_path = path | ||
.path() | ||
.strip_prefix(documents_path) | ||
.map_err(|e| CreationError(e.to_string()))?; | ||
|
||
let str_path = path.path().to_string_lossy().to_string(); | ||
if !Self::checksum_is_equal(&str_path, existing_checksum).unwrap_or(false) { | ||
println!("Adding document: {}", relative_path.to_string_lossy()); | ||
message_display.display(Message::Info(&format!("Adding document: {}", relative_path.to_string_lossy()))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comment in in main.rs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See my comment above :)
No description provided.