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(helper): add more external help providers to plz menu #56

Closed
wants to merge 34 commits into from

Conversation

0x61nas
Copy link
Contributor

@0x61nas 0x61nas commented Jun 22, 2023

Description

Add eg, cheat, and Command Line Interface Pages as options to the plz menu

I actually added eg only, 'cause it's obvious that we need a uniform way to handle all these options + cheat.sh to reduce the code duplication, but I opened the PR anyway to track the progress and discuss the changes.

Motivation and Context

How Has This Been Tested?

  • By run cargo t
  • By using the plz menu

Types of Changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation (no code change)
  • Refactor (refactoring production code)
  • Other

Checklist:

  • My code follows the code style of this project.
  • I have updated the documentation accordingly.
  • I have formatted the code with rustfmt.
  • I checked the lints with clippy.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@orhun
Copy link
Owner

orhun commented Jun 23, 2023

PR looks good! I think we can follow an approach like this for avoiding code duplication: (rough sketch)

trait InfoProvider {
    // Return the URL of the cheat sheet provider.
    fn url(&self) -> &str;

    // Show the cheat sheet for the given command.
    fn show_info<Output: Write>(
        &self,
        cmd: &str,
        pager: &Option<String>,
        output: &mut Output,
    ) -> Result<()> {
        let client = AgentBuilder::new().build();
        let cheat_sheet = client
            .get(&format!("{}/{}", self.url(), cmd))
            .call()
            .map_err(|e| Error::from(Box::new(e)))?
            .into_string()?;
        // Don't use a pager when the topic is not found.
        if let Some(pager) = pager
            .as_ref()
            .filter(|_| !cheat_sheet.starts_with("Unknown topic."))
        {
            let mut process = if cfg!(target_os = "windows") {
                Command::new("cmd")
                    .args(["/C", pager])
                    .stdin(Stdio::piped())
                    .spawn()
            } else {
                Command::new("sh")
                    .args(["-c", pager])
                    .stdin(Stdio::piped())
                    .spawn()
            }?;
            if let Some(stdin) = process.stdin.as_mut() {
                writeln!(stdin, "{}", cheat_sheet)?;
                process.wait()?;
            }
        } else {
            writeln!(output, "{}", cheat_sheet)?;
        }
        Ok(())
    }
}

struct CheatSh {}

impl InfoProvider for CheatSh {
    fn url(&self) -> &str {
        "https://cheat.sh"
    }
}

struct EgPages {}

impl InfoProvider for EgPages {
    fn url(&self) -> &str {
        "https://raw.githubusercontent.com/srsudar/eg/master/eg/examples"
    }
}

@0x61nas
Copy link
Contributor Author

0x61nas commented Jun 24, 2023

I think we can follow an approach like this for avoiding code duplication: (rough sketch)

It pretty reasonable approach

# Conflicts:
#	src/helper/docs/mod.rs
@codecov-commenter
Copy link

codecov-commenter commented Jun 24, 2023

Codecov Report

Patch coverage: 64.83% and project coverage change: -1.39% ⚠️

Comparison is base (f644476) 77.16% compared to head (e6e8484) 75.76%.
Report is 2 commits behind head on main.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #56      +/-   ##
==========================================
- Coverage   77.16%   75.76%   -1.39%     
==========================================
  Files          14       16       +2     
  Lines         604      693      +89     
==========================================
+ Hits          466      525      +59     
- Misses        138      168      +30     
Files Changed Coverage Δ
src/error.rs 100.00% <ø> (ø)
src/helper/docs/mod.rs 15.56% <22.96%> (+15.56%) ⬆️
src/cli.rs 82.54% <71.43%> (+0.09%) ⬆️
src/helper/docs/cheat_sh.rs 93.11% <93.11%> (ø)
src/config.rs 70.38% <100.00%> (+1.95%) ⬆️
src/helper/docs/cheatsheets.rs 100.00% <100.00%> (ø)
src/helper/docs/eg.rs 100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@orhun
Copy link
Owner

orhun commented Jul 18, 2023

Hey @0x61nas, is this still on your list?

@0x61nas
Copy link
Contributor Author

0x61nas commented Jul 19, 2023

yes, I'll try to make progress in this PR as soon as I can

@orhun
Copy link
Owner

orhun commented Jul 20, 2023

Alright, let's have this one ready for the next release!

# Conflicts:
#	src/helper/docs/mod.rs
The 'cheat' module was renamed to 'cheat_sh'. The import statement in 'config.rs' is updated to reflect this new name. This change ensures the consistency in module naming and prevents import errors.
The error handling function `err_handle` in the `docs` module was refactored. Instead of simply returning a `Result<String>`, the function now returns an `Error` with two branches. The first branch handles the case where the error is of type `ureq::ErrorKind::HTTP`, returning a custom `ProviderError`. The second branch simply constructs an `Error` from the original error. This change provides more flexibility in handling different types of errors, increasing code robustness.
Changed the 'cheat_sh_url' in the config from a mandatory String to an optional String type. This will allow users to decide whether they want to provide a custom URL for cheat.sh or not, increasing flexibility in configuration options.
…to the `_fetch` function for better custom URL handling
@0x61nas
Copy link
Contributor Author

0x61nas commented Jul 30, 2023

Hi @orhun, I created the initial form for the HelpProvider trait and I implemented it for both cheat.sh and eg pages as a start to see if the code actually works.

but I wanna make sure that the HelpProvider interface is reasonable and stable so I hope that you can take a look at it and share your suggestions and thoughts. so I can modify it now before I implement the rest of the suggestions in #14.

and sorry for the delay.

@orhun
Copy link
Owner

orhun commented Jul 31, 2023

Looks good, let's proceed with the implementation! 🐻

and sorry for the delay.

Not an issue!

@0x61nas 0x61nas closed this Aug 1, 2023
@0x61nas 0x61nas deleted the add-more-options-to-plz-menu branch August 1, 2023 18:21
@0x61nas 0x61nas restored the add-more-options-to-plz-menu branch August 1, 2023 18:27
@0x61nas 0x61nas reopened this Aug 1, 2023
@0x61nas 0x61nas changed the title Add more options to plz menu feat: add more external help providers to please menu Aug 1, 2023
@orhun
Copy link
Owner

orhun commented Aug 2, 2023

Let me know when this is ready for review!

@0x61nas 0x61nas marked this pull request as ready for review August 9, 2023 19:53
@orhun orhun self-requested a review August 11, 2023 12:10
@0x61nas 0x61nas changed the title feat: add more external help providers to please menu feat: add more external help providers to plz menu Aug 11, 2023
@orhun orhun changed the title feat: add more external help providers to plz menu feat(helper): add more external help providers to plz menu Aug 17, 2023
Copy link
Owner

@orhun orhun left a comment

Choose a reason for hiding this comment

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

Thanks! This is now squashed and merged in adadc2e

@orhun orhun closed this Aug 17, 2023
@0x61nas 0x61nas deleted the add-more-options-to-plz-menu branch August 18, 2023 10:41
@0x61nas 0x61nas mentioned this pull request Sep 15, 2023
14 tasks
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.

Add the ability to use more types of manuals/cheatsheets
3 participants