fix: Vivaldi installed via snap not working

[E1Bos]

Hello again!

I made a couple of changes.

9305ea9 is some general changes/fixes:

• Type hints and docstrings were added to all functions
• A configured logger was added (and commented out) which will log directly to debug.log
• The watch-and-deploy script was changed to ignore that file.
• os.popen was closed after use

894a9d5 is the fix to #6.

• the unused ItemEnterEvent import was removed

ebca8f9 is a small refactor of find_bookmarks_paths to reduce code duplication and increase legibility. All searched paths are stored in a list, and it iterates through it and calls find | grep Bookmarks on each.

Everything else is fixing mistakes on my part or issues with GitHub 😁

Regarding formatting, I formatted with ruff (which formats like Black), which changed the single quotes to double quotes, so I apologise if you're not too pleased with that.

Testing on my machine with both the snap and .deb version of Vivaldi, bookmarks are fetched from both browsers. Please let me know if you'd like me to make any changes.

[pascalbe-dev]

@E1Bos

Thanks a lot for the next contribution! I had a quick look, but will look closer soon.

My first look feedback:

• very great, that you split changes by commits. That makes understanding your thought process and also reviewing in general so much easier
• the type hints are great 🏅
• I have no strong opinion about the doc strings. Can do that.
• the logging ideas are great. Will test that soon, but the idea is great
• the formatting is also fine

Does this formatter also allow checking and fixing the formatting via a script? Then we could add a github action to check the formatting on pull request. I'm not super familiar with all this python stuff. You seem to be more familiar, so if you have experience, feel free to either add scripts or list these steps in the readme for local development.

This is not necessary for the merge though. So I would also merge without that. Just thought, that this would make sense.

I'll let you know, if I miss something when I reviewed in detail and also tested.

Thanks again for the contribution 🎖️

[E1Bos]

@pascalbe-dev

Thank you for the feedback!

Ruff does support checking and fixing via a script (when running in the terminal ruff check and ruff check --fix do those actions), as well as the formatting, ruff format.

There is a GitHub action available, however I am not familiar with github actions. I tried to use them once and ended up with something like 40 commits trying to debug and test it since I couldn't get the testing env working locally. If you have any experience, your input would be much appreciated.

Regarding your other suggestions, I'd be more than happy to make some changes to the readme regarding local development, and will do that later today.

[pascalbe-dev]

@E1Bos

Perfect, don't worry about the github actions. I'll add that some time later then :)

[E1Bos]

@pascalbe-dev

I was looking over the readme and I can't seem to figure out what else to add to the local development section. Someone looking over the code will immediately see the comment, so I don't think its necessary to add it to the readme. Plus, you did a really good job covering the steps already.

If you're happy with the commit, I would just merge it.

[E1Bos]

@pascalbe-dev @cristianoliveira

I just added 810f1b7 to the PR, which addresses #4

• If the env variable CUSTOM_BROWSER_FOLDER is set, it'll search the specified dirs (separated by :)
  • Custom paths will show up with the chrome icon
• The logic to grep Bookmarks was moved to its own static method collect_bookmarks_paths so that it can be used both to find supported browsers and the custom browsers properly
• Logs were slightly adjusted and the commented-out logging line was improved

I'm curious to hear your feedback. I wasn't quite sure what to name the env variable, so please let me know if you have any suggestions. Also, it could make sense to switch the custom icon to the chromium icon instead of chrome, again, please let me know.

I've been looking into it, and I feel it may be more convenient to have a text field setting in the browser extension itself instead of using an env variable. I'll try to implement that later. Would still love to hear your thoughts.

[E1Bos]

Finished this last little bit.

Here are the changes in f874d20:

• Added PreferencesEventListener class
  • PreferencesEvent is sent when the module first runs
    • This reads the current preferences and sets it to self.preferences, which is accessed in find_bookmarks_paths
  • PreferencesUpdateEvent is sent whenever the settings are saved, does basically the same thing
  • find_bookmarks_paths is called after self.preferences is updated. On start (because of from PreferencesEvent) and whenever preferences are updated, which will grab any bookmarks from the new path
  • find_bookmarks_paths is no longer a static method, since it needs to access self.preferences. As a result, __init__ was modified to run the super method first
• manifest.json was updated to have the new additional_browser_paths setting
• Changed custom browser icon to chromium

I prefer this approach to an env variable since you don't need to remember a var name 😁

[pascalbe-dev]

@E1Bos

Thanks! I'll check it out soon.

[pascalbe-dev]

Looks good to me. Merging. Thanks for the addition :)

Great to have the type hints in there also :)

[pascalbe-dev]

By the way, @E1Bos , in case you are interested. Here I added the github action to run formatting and linting: build: add formatting and linting

Thanks for pointing to the ruff tool by the way. Works well :)

[E1Bos]

Hey @pascalbe-dev,

Sorry for the late reply. Thank you for merging, I was more than happy to contribute to the project, I can't express how incredibly convenient this extension is. I looked over the github action, very glad you figured out how to get it work, happy it works well :)

I have other ideas for ways to add to this extension but can't seem to figure them out for now (at least with Vivaldi)

Thank you for your work and continued maintenance!