NurioHin / mpv-bookmarker

A bookmarker menu to manage all your bookmarks in MPV
97 stars 24 forks source link

special/unicode characters in filename disrupt loading #3

Closed elig0n closed 3 years ago

elig0n commented 4 years ago

special unicode characters or foreign language character sets such as 🛑 or Č that appears in filenames prevents letting the script load the file i.e. Can't find file for bookmark ....

NurioHin commented 4 years ago

I cannot replicate this bug. As a test, I named a file Č.mkv and bookmarked it, closed mpv, and then loaded that bookmark, and everything worked fine. Are you using Windows, and if so, is your Windows codepage set to UTF-8? You can check by running cmd.exe and executing chcp. The codepage should be 65001

elig0n commented 4 years ago

Code page is different, but even after running chcp 65001 in cmd and running mpv right after it the error remains (I bookmarked new and then tried to load it).

NurioHin commented 4 years ago

I am not sure what to tell you. It seems to be working fine on my system:

image

And I don't mean that in a way of "Works on my system; it's your problem". But if I can't even replicate this problem, it'll be impossible for me to fix it, as much as I'd want to

mochaaP commented 3 years ago

I can replicate this problem too.

winver.exe mpv.net

Running on Windows 10 2004 zh-CN, default codepage is 936, setting it to 65001 doesn't make it work either.

A tempfix (replace the original function) that works for me:

function fileExists(path)
  local f = utils.file_info(path)
  return f.is_file
end

I guess we can rewrite the path related helper functions, as they are old and dirty (e.g. getFilepath() will only return %AppData%/mpv but not portable_config)?

elig0n commented 3 years ago

@NurioHin ^

mochaaP commented 3 years ago

@elig0n Hey, thanks for the notification, just rewrote some helper functions (logic from https://github.com/VideoPlayerCode/mpv-tools, I just rewrote them in Lua) and here's a patch:

-- // General utilities \\ --

local _isUnix, _isMac, _pathSep

function _detectOS() -- from VideoPlayerCode/modules.js/PathTools.js
  -- Detect Unix/Linux/macOS if the path starts with a forward slash.
  _isUnix = string.sub(utils.getcwd(), 1, 1) == '/'
  _isMac = false -- Mac is also Unix, but we'll detect separately.
  _pathSep = _isUnix and '/' or '\\'

  if _isUnix then
    local unameResult = utils.command_native({
      name = "subprocess",
      args = {"uname", "-s"},
      cancellable = false,
      capture_stdout = true
    })
    if unameResult:match "^%s*Darwin%s*$" then
      _isMac = true
    end
  end
end

-- Check if the operating system is Mac OS
function isMac()
  if _isMac == nil then
    _detectOS()
  end
  return _isMac
end

-- Check if the operating system is Windows
function isWindows()
  if _isUnix == nil then
    _detectOS()
  end
  return not _isUnix
end

-- Check whether a certain file exists
function fileExists(path)
  local f = utils.file_info(path)
  return f.is_file
end

-- Get the filepath of a file from the mpv config folder
function getFilepath(filename)
  return mp.command_native({"expand-path", "~~/" .. filename})
end

Hope it works ;)

mochaaP commented 3 years ago

Ready for review.