Work in Progress: Initial implementation of --doctor command
#1193
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
clnoll
force-pushed
the
doctor
branch
5 times, most recently
from
335a0cc
to
3220722
Compare
clnoll
force-pushed
the
doctor
branch
3 times, most recently
from
11898ce
to
369a044
Compare
dandavison
reviewed
Nov 23, 2022
src/subcommands/doctor/less.rs
Outdated
| impl Diagnostic for Less { | ||
| fn report(&self) -> (String, bool) { | ||
| match self.version { | ||
| Some(n) => match n < self.min_version { |
There was a problem hiding this comment.
This could alternatively use
Some(n) if n < self.min_version
src/subcommands/doctor/report.rs
Outdated
| let mut reports = Vec::new(); | ||
| for d in diagnostics { | ||
| reports.push(build_report_row(d)); | ||
| } |
There was a problem hiding this comment.
More code golf; I think an alternative here is
let reports = diagnostics.into_iter().map(build_report_row).collect();
src/subcommands/doctor/report.rs
Outdated
| Ok(()) | ||
| } | ||
|
|
||
| fn build_report_row(diagnostic: Box<dyn Diagnostic>) -> Report { |
There was a problem hiding this comment.
This could be named build_report maybe since it returns Report? Or, should it be Report::from_diagnostic()
There was a problem hiding this comment.
Thanks, I went with the Report::from_diagnostic() version.
src/subcommands/doctor/report.rs
Outdated
| use tabled::{Style, Table, Tabled}; | ||
|
|
||
| #[cfg(not(tarpaulin_include))] | ||
| pub fn run_diagnostics() -> std::io::Result<()> { |
There was a problem hiding this comment.
Maybe this should be called print_doctor_report since it's the entrypoint.
|
Thanks for the review @dandavison! I made the updates you suggested. |
Provides the structure for adding checks to the user's environment that might interfere with delta usage. Also includes checks for - `less` version - `GIT_CONFIG*` environment variables - `*PAGER` environment variables
|
Rebased on master |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is an initial version of the
--doctorcommand, which will check various settings of a user's environment and identify possible incompatibilities withdelta.This PR is intended to show a potential pattern for adding new
--doctorchecks, so I've intentionally kept the list of handled checks short, in anticipation of structural changes that are suggested during code review.The suggested pattern for adding new checks is to add a new
structthat implements theDiagnostictrait, comprised of the following methods:diagnose: gets the setting in the user's environment, and compares it to allowed settings. Returns aHealthobject with a diagnosis & remedy if the setting is potentially incompatible with delta.remedy: Suggests a remedy, if theHealthobject returned bydiagnoseindicates that the setting may be incompatible.report: Reports the value of the setting, whether or not it is "healthy".Currently,
delta --doctorprints thereportto stdout.Addresses #1184.