Work in Progress: Initial implementation of --doctor command#1193
Open
clnoll wants to merge 9 commits into
Open
Work in Progress: Initial implementation of --doctor command#1193clnoll wants to merge 9 commits into
--doctor command#1193clnoll wants to merge 9 commits into
Conversation
335a0cc to
3220722
Compare
11898ce to
369a044
Compare
dandavison
reviewed
Nov 23, 2022
| impl Diagnostic for Less { | ||
| fn report(&self) -> (String, bool) { | ||
| match self.version { | ||
| Some(n) => match n < self.min_version { |
Owner
There was a problem hiding this comment.
This could alternatively use
Some(n) if n < self.min_version| let mut reports = Vec::new(); | ||
| for d in diagnostics { | ||
| reports.push(build_report_row(d)); | ||
| } |
Owner
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();| Ok(()) | ||
| } | ||
|
|
||
| fn build_report_row(diagnostic: Box<dyn Diagnostic>) -> Report { |
Owner
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()
Contributor
Author
There was a problem hiding this comment.
Thanks, I went with the Report::from_diagnostic() version.
| use tabled::{Style, Table, Tabled}; | ||
|
|
||
| #[cfg(not(tarpaulin_include))] | ||
| pub fn run_diagnostics() -> std::io::Result<()> { |
Owner
There was a problem hiding this comment.
Maybe this should be called print_doctor_report since it's the entrypoint.
Contributor
Author
|
Thanks for the review @dandavison! I made the updates you suggested. |
Owner
|
Rebased on master |
Owner
|
Another rule possibility: check for installed bat version that would create incompatible assets. Ref #1906 |
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
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.