-
Notifications
You must be signed in to change notification settings - Fork 14
feat: added browser and profile functions #19
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
Conversation
|
PR #17 has more info on this |
|
Can you add support for other browsers as well, ideally the major browsers (Firefox, Brave, Edge) |
|
@codex review deeply |
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let info_cache = json_value | ||
| .get("profile") | ||
| .and_then(|p| p.get("info_cache")) | ||
| .and_then(|ic| ic.as_object()) | ||
| .ok_or_else(|| "Could not find 'profile' or 'info_cache' in JSON.")?; |
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.
Use an error type compatible with
Box<dyn Error>
In get_chrome_profiles, the ok_or_else call returns an Err(&str), but the function’s error type is Box<dyn std::error::Error>. The ? operator cannot convert a plain string slice into a boxed error, so the function will not compile once this module is referenced. Wrap the message in a concrete error type (e.g., io::Error) or box it before returning.
Useful? React with 👍 / 👎.
|
@AdityaVKochar resolve the error type compatibility |
|
To use Codex here, create a Codex account and connect to github. |
|
@theg1239 added for chromium based browsers and firefox |
|
@codex review |
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let base_dir = if cfg!(target_os = "windows") || cfg!(target_os = "macos") { | ||
| data_local_dir() | ||
| } else { | ||
| dirs::home_dir() |
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.
Firefox profiles looked up under Local instead of Roaming AppData
On Windows the function builds the base directory using dirs::data_local_dir() before appending Mozilla\Firefox\Profiles, but Firefox stores profiles under %APPDATA% (Roaming) rather than Local. With the current path the directory almost always does not exist, so the function returns an empty list even when profiles are present. Use dirs::data_dir() or the roaming path instead.
Useful? React with 👍 / 👎.
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.
@AdityaVKochar can you verify this?
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.
works for both local and roaming
tested
| #[cfg(target_os = "linux")] | ||
| path.push("mozilla-firefox/"); |
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.
Linux Firefox profile directory is mis-specified
For Linux the code appends "mozilla-firefox/" beneath the user’s home directory, but Firefox stores its profiles under ~/.mozilla/firefox. Because of the missing leading dot and incorrect directory name, the constructed path never exists and no profiles are discovered. Adjust the path to ~/.mozilla/firefox.
Useful? React with 👍 / 👎.
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.
@AdityaVKochar this as well along with the other issue
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.
fixed the paths
#8
Added functions