Wrong Next File
Eisa01 opened this issue · 20 comments
Hello Jonnie,
I must say, impressive work with next file. Wanted to report some wrong next file issues I have encountered using it.
Lets take Naruto for example:
Naruto E09 > Naruto E10 > Naruto E100 > 101 until 109 is correct > Naruto E11
That gives you an idea of how it looks at the numbers, however when a folder has even more episodes, it becomes a total mess.
Lets take Detective Conan for example:
Detective Conan E01 > Detective Conan E260 > Detective Conan E367 > Detective Conan E371 > And the next file will continue jumping to wrong episodes almost randomly.
I hope this gives you an idea.
Thanks again for this awesome script!
Regards,
Eisa
Thanks for the issue. What operating system do you use?
Windows 10 OS
Sorry for not mentioning earlier.
Ok, the problem is that this script uses dir /b to list the files in a directory. It seems the order is different from what you would expect in the file explorer. I will look into how to list the files in the correct order.
Interesting. I will be awaiting your response ^ ^
Had sometime to play around a bit..
Powershell listing is possible to list the directories in the correct order
If you replace dir /b, shown below:
Line 60 in 2d94330
to:
local script = [[
powershell -Command "(Get-ChildItem | Sort-Object { [regex]::Replace($_.Name, '\d+', { $args[0].Value.PadLeft(20) }) }).Name"
]]
popen, err = io.popen(script..(search:gsub("/", "\\")))
It will list correctly, however could be slow in large directories, and wont get path in some cases.
Probably io.popen is causing the slowness issue, so if using subprocess, I believe this can be achieved as fast as dir /b and without cmd prompt appearing,
I wrote a powershell function to showcase this utilizing your return handlers, the function should work fine.
function handleres(res, args)
if not res.error and res.status == 0 then
return res.stdout
else
msg.error(" Status: "..(res.status or ""))
msg.error(" Error: "..(res.error or ""))
msg.error(" stdout: "..(res.stdout or ""))
msg.error("args: "..utils.to_string(args))
return ''
end
end
function get_list()
local args = {
'powershell', '-NoProfile', '-Command', [[& {
Trap {
Write-Error -ErrorRecord $_
Exit 1
}
$list = (Get-ChildItem | Sort-Object { [regex]::Replace($_.Name, '\d+', { $args[0].Value.PadLeft(20) }) }).Name
$u8list = [System.Text.Encoding]::UTF8.GetBytes($list)
[Console]::OpenStandardOutput().Write($u8list, 0, $u8list.Length)
}]]
}
return handleres(utils.subprocess({ args = args, cancellable = false }), args)
end
This function will get the list of files, and can be set to a variable
local list = get_list()
Thanks, this looks promising. I will play around with it later today.
Had to update the powershell to this monster:
cd ]]..path..[[
$list = (Get-ChildItem -Attributes !Directory+!System | Sort-Object { [regex]::Replace($_.Name, '\d+', { $args[0].Value.PadLeft(20) }) }).Name
$console = [Console]::OpenStandardOutput()
ForEach ($line in $($list -split "`r`n"))
{
$u8list = [System.Text.Encoding]::UTF8.GetBytes($line)
$console.Write($u8list, 0, $u8list.Length)
$newline = [System.Text.Encoding]::UTF8.GetBytes([system.environment]::newline)
$console.Write($newline , 0, $newline.Length)
}If you have any idea how to keep the newlines of $list when outputting let me know.
In the meanwhile, I need to update the logic a bit on the linux side as well. Will probably commit something tomorrow.
Perhaps this will help.
To list them in new lines, removed the UTF8 conversion, simply outputted the same way powershell displays it, each file would be in new line:
Also used pwd to get the path in powershell since this is what you are using in the script
function get_list()
local args = {
'powershell', '-NoProfile', '-Command', [[& {
Trap {
Write-Error -ErrorRecord $_
Exit 1
}
cd "]]..pwd..[["
$list = (Get-ChildItem | Sort-Object { [regex]::Replace($_.Name, '\d+', { $args[0].Value.PadLeft(20) }) }).Name
Write-Output $list
}]]
}
return handleres(utils.subprocess({ args = args, cancellable = false }), args)
end
The issue with Write-Output $list seems to be that it only works for some subset of characters. Filenames containing characters like ä or ü do not seem to work.
The dir variable is path.join(pwd, relative_path) so that the script still works if mpv was launched like this mpv files/media1.mp4.
edit:
Get-Content by default returns an array of strings where each element represents a line. When you pass that array to .NET's regex replace method, PowerShell doesn't see a method overload that takes a string array but does see one that takes a string, so it coerces your string array into a string. You can see the same effect if you do this right after the Get-Content call
Get-Content -Raw filename will return a single string rather than an array of per-line strings
This might be the same issue. Will investigate a bit more later. I have finished updating and testing the linux part so I will merge it once I verify the windows version still works.
Ah I see, things make more sense for me now.
Thanks for the clarifications
Having said that, I still don't have a clear picture on how you need the files to be outputted.
Maybe having one output, in it each file is seperated by quotes could be useful for you?
function get_list()
local args = {
'powershell', '-Command', [[& {
Trap {
Write-Error -ErrorRecord $_
Exit 1
}
$list = (Get-ChildItem -Attributes !Directory+!System | Sort-Object { [regex]::Replace($_.Name, '\d+', { $args[0].Value.PadLeft(20) }) }).Name
$console = [Console]::OpenStandardOutput()
ForEach ($line in $($list -split "`r`n"))
{
$line = "`"$line`" "
$u8list = [System.Text.Encoding]::UTF8.GetBytes($line)
$console.Write($u8list, 0, $u8list.Length)
}
}]]
}
return handleres(utils.subprocess({ args = args, cancellable = false }), args)
end
Ah I finally figured it out
$list = (Get-ChildItem -File | Sort-Object { [regex]::Replace($_.Name, '\d+', { $args[0].Value.PadLeft(20) }) }).Name
$string = ($list -join "`r`n")
$u8list = [System.Text.Encoding]::UTF8.GetBytes($string)
[Console]::OpenStandardOutput().Write($u8list, 0, $u8list.Length)I guess the ideal would be to separate with a character that is illegal in filenames, which would make splitting guaranteed to be correct. However, since newlines are so incredibly rare in filenames I'm ok for at least now to just split by new lines. This avoids escaping and other annoying gotchas.
New commit 2037030
I must say.. Impressive work!!
I guess the next challenge would be to seperate only on illegal filename.
The only illegal character for both linux/unix and windows is
/ (forward slash)
So it will be most ideal to seperate using this.
More details according to:
https://stackoverflow.com/questions/1976007/what-characters-are-forbidden-in-windows-and-linux-directory-names
The forbidden printable ASCII characters are:
Linux/Unix:
/ (forward slash)
Windows:
< (less than)
> (greater than)
: (colon - sometimes works, but is actually NTFS Alternate Data Streams)
" (double quote)
/ (forward slash)
\ (backslash)
| (vertical bar or pipe)
? (question mark)
* (asterisk)
Forward slash definitely seems like the best choice so the splitting logic can be shared. It's a trivial change for windows $string = ($list -join "/") but the linux version needs some work.
Currently there is a bug preventing from making the new commit usable,
Only in some rare cases, the paths are accepting the powershell code.
Mostly the script is breaking, you can see the error log below.
I will debug this for a while, since when it works its magical with the new commet ^ ^
[nextfile] Trap {
[nextfile] Write-Error -ErrorRecord $_
[nextfile] Exit 1
[nextfile] }
[nextfile] cd C:\91 Days/
[nextfile] $list = (Get-ChildItem -File | Sort-Object { [regex]::Replace($_.Name, '\d+', { $args[0].Value.PadLeft(20)
[nextfile] }) }).Name
[nextfile] $string = ($list -join "`r`n")
[nextfile] $u8list = [System.Text.Encoding]::UTF8.GetBytes($string)
[nextfile] [Console]::OpenStandardOutput().Write($u8list, 0, $u8list.Length)
[nextfile] : A positional parameter cannot be found that accepts argument 'Days/'.
[nextfile] At line:1 char:1
[nextfile] + & {
[nextfile] + ~~~
[nextfile] + CategoryInfo : InvalidArgument: (:) [Write-Error], ParameterBindingException
[nextfile] + FullyQualifiedErrorId : PositionalParameterNotFound
[nextfile]
[nextfile]
[nextfile] stack traceback:
[nextfile] C:/Portable Programs/mpv/scripts/nextfile.lua:34: in function 'fn'
[nextfile] mp.defaults:225: in function 'fn'
[nextfile] mp.defaults:60: in function 'handler'
[nextfile] mp.defaults:365: in function 'handler'
[nextfile] mp.defaults:486: in function 'call_event_handlers'
[nextfile] mp.defaults:520: in function 'dispatch_events'
[nextfile] mp.defaults:479: in function <mp.defaults:478>
[nextfile] [C]: at 0x00fe7fa0
[nextfile] [C]: at 0x00fe7770
AV: 00:00:00 / 00:24:27 (0%) A-V: 0.000
[nextfile] Lua error: C:/Portable Programs/mpv/scripts/nextfile.lua:98: attempt to concatenate local 'error' (a nil value)
Thanks, I can also take a look if you provide paths/filenames to reproduce
edit: ah it seems like the cd path should be quoted to support spaces. In windows filenames cannot have quotes so the path doesn't need escaping. Fixed in 7dc8660
Too fast ^ ^. Thanks =)
I added support and tested out newline files in linux. I also fixed some issues from dir..file concat, with path_join the path should always be correct.
Update ad118b9
This is closing to perfection..
Just if people knew how much thought goes into a line of code.
In the commit I can also see that forward slash is utilizied.
Does that mean it is done?
Yes i changed the split to be by forward slash to support files with newlines. I couldn't create an actual file with a newline in windows but at least on linux it works. I think it's done for now. Let me know if you ecounter any issues with the script. I also adapted this powershell script for playlistmanager script since it's a lot better than using popen. Thanks for your help!
Actually, seems like I didn't verify linux sort order. With find command the files are no longer sorted naturally so I reverted linux back to newline implementation for now. I kept windows joined by forward slash by passing a delimiter string that will be used for splitting the files up.
I created a new issue to track the linux bug #8