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: support opening the first HTTP reference in browser (#11) #15

Merged
merged 4 commits into from
May 24, 2024

Conversation

mertssmnoglu
Copy link
Sponsor Contributor

Description of change

Users will now be able to open CVE reference URL's directly in their browsers via pressing Space (#11)

Add new Pattern to handling the Space character. If user pressed the Space key on the details, look for all references starting with 'http' and open the first URL in the web browser if available.

New Dependency

Add webbrowser dependency to open browser on cross platform. 1026380

How has this been tested?

🟢 Run all tests

running 3 tests
test error::tests::test_error ... ok
test widgets::tests::test_selectable_list ... ok
test args::tests::test_args ... ok

Manual Test Cases

CVE has no references | (1/3)

  • Expected Behaviour: Do nothing
  • Test CVE: CVE-1999-0020

cve-1999-0020-on-flawz

  • I tested this case and it works as expected.

CVE has reference without any http URL | (2/3)

cve-1999-0044-on-flawz

  • I tested this case and it works as expected.

CVE has reference with http URL | (3/3)

  • Expected Behaviour: Open web browser with given URL
  • Test CVE: CVE-1999-0116

cve-1999-0116-on-flawz

  • I tested this case and it works as expected.

Known Linter Warnings

warning: used `unwrap()` on an `Option` value
  --> src/handler.rs:43:35
   |
43 |                 let references = &app.list.selected().unwrap().references;
   |                                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = note: if this value is `None`, it will panic
   = help: consider using `expect()` to provide a better panic message
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unwrap_used

This is my first rust experience. Please feel free to tell me if you see anything wrong in this PR.

@mertssmnoglu mertssmnoglu requested a review from orhun as a code owner May 20, 2024 22:13
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.

Thank you for the PR! It looks good to me overall. Just had one comment about refactoring.

Also, can you update README.md about this new key binding?

src/handler.rs Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

codecov-commenter commented May 21, 2024

Codecov Report

Attention: Patch coverage is 0% with 11 lines in your changes are missing coverage. Please review.

Project coverage is 8.14%. Comparing base (f9a94ea) to head (050a56b).
Report is 3 commits behind head on main.

Files Patch % Lines
src/handler.rs 0.00% 11 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@           Coverage Diff            @@
##            main     #15      +/-   ##
========================================
- Coverage   8.27%   8.14%   -0.13%     
========================================
  Files         13      13              
  Lines        701     712      +11     
========================================
  Hits          58      58              
- Misses       643     654      +11     

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

mertssmnoglu and others added 2 commits May 21, 2024 21:55
- improve code simplicity
- better error printing with eprintln

Co-authored-by: Orhun Parmaksız <orhunparmaksiz@gmail.com>
@mertssmnoglu
Copy link
Sponsor Contributor Author

mertssmnoglu commented May 21, 2024

Thank you for the PR! It looks good to me overall. Just had one comment about refactoring.

Also, can you update README.md about this new key binding?

I got your changes with fixings.
Currently I'm not into writing a user guide for this change. I think the owner of the repo(you) will describe your project better than me.

@orhun
Copy link
Owner

orhun commented May 22, 2024

We can continue with this PR. You can simply update the key bindings table to mention this new functionality and I think that will be enough.

@mertssmnoglu
Copy link
Sponsor Contributor Author

We can continue with this PR. You can simply update the key bindings table to mention this new functionality and I think that will be enough.

I couldn't see available key bindings table in README.md so I add a new table. #20

@orhun
Copy link
Owner

orhun commented May 24, 2024

Oops, I thought I added it 😅 Thanks for the PR!

@orhun orhun merged commit e970ebc into orhun:main May 24, 2024
12 checks passed
@orhun orhun changed the title feat: support opening first http reference in browser (#11) feat: support opening the first HTTP reference in browser (#11) May 24, 2024
This was referenced May 24, 2024
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.

3 participants