jarun / buku

:bookmark: Personal mini-web in text
GNU General Public License v3.0
6.52k stars 294 forks source link

Auto import from Microsoft Edge not working on Windows 11 #682

Closed dasbootbee closed 1 year ago

dasbootbee commented 1 year ago

PS C:\Users\user> buku -g --ai [DEBUG] buku v4.8 [DEBUG] Python v3.10.11 [DEBUG] get_firefox_profile_name(): C:/Users/first name/AppData/Roaming/Mozilla/Firefox/ does not exist Generate auto-tag (YYYYMonDD)? (y/n): n Add parent folder names as tags? (y/n): y Import bookmarks from google chrome? (y/n): n Import bookmarks from chromium? (y/n): n Import bookmarks from Vivaldi? (y/n): n Import bookmarks from Firefox? (y/n): n Import bookmarks from microsoft edge? (y/n): y [ERROR] Could not import bookmarks from microsoft-edge

I'm wondering if the way you're constructing the user profile directory on Windows is correct. On Windows, your user profile directory is an abbreviated mix of your first name and last name. But in the debug output above, it's pulling the first name (denoted in bold) as the User directory and that isn't correct. I don't know if it's doing the same for pulling the bookmarks file for Edge but I thought I'd mention it.

LeXofLeviafan commented 1 year ago

I'm wondering if the way you're constructing the user profile directory on Windows is correct.

…Incidentally, it should be possible to solve this sort of problem once-and-for-all by relying on a dedicated library (e.g. platformdirs)

jarun commented 1 year ago

We prefer to keep the dependencies thin.

@dasbootbee where is the directory located?

@dertuxmalwieder can you have a look please?

dertuxmalwieder commented 1 year ago

On Windows, your user profile directory is an abbreviated mix of your first name and last name.

Not necessarily. I’ll investigate (in a few hours, probably). I don’t think this requires a separate library.

edit: Right now, Buku uses os.path.expanduser to determine the local directories as far as I (can see and) remember. I think that’s a good approach. I will have a closer look when I’m in the office again.

LeXofLeviafan commented 1 year ago

Problem is, the DIY approach is easily breakable since user folder locations aren't fixed in stone.

The implementation in said library is far from trivial (and quite large, due to including 2 fallback approaches in case the better one(s) isn't available).

jarun commented 1 year ago

We don't want to introduce library deps. The alternative to import manually is available.

dertuxmalwieder commented 1 year ago

From the Python documentation on what Buku currently uses:

On Windows, USERPROFILE will be used if set, otherwise a combination of HOMEPATH and HOMEDRIVE will be used.

It works just fine on this computer (Windows 10) where USERPROFILE is set.

@dasbootbee Just curious: What do $env:USERPROFILE and $env:CSIDL_PROFILE print from inside your PowerShell?

dasbootbee commented 1 year ago

$env:USERPROFILE = first three of first name and two letters from last name. That is my correct userprofile directory on Windows. $env:CSIDL_PROFILE is not set

dertuxmalwieder commented 1 year ago

Ah, makes sense. Possible patch attached.

diff.patch

jarun commented 1 year ago

@dasbootbee can you please confirm if the patch works?

dasbootbee commented 1 year ago

Auto import seems to be working with the patch applied.