Skip to content
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

key bind for list of delete marks #27

Merged
merged 7 commits into from
Mar 7, 2021
Merged

key bind for list of delete marks #27

merged 7 commits into from
Mar 7, 2021

Conversation

bitingsock
Copy link
Contributor

#14

@bitingsock
Copy link
Contributor Author

Also, if I can get permissions I'd like to help clear the issue tracker.

@zenyd
Copy link
Owner

zenyd commented Mar 6, 2021

How does this look if the file list to be deleted is so long that it wouldn't fit in the window? Maybe delimiting by ';' instead of newlines might be better plus printing also to console.

@bitingsock
Copy link
Contributor Author

good point. take a look

@zenyd
Copy link
Owner

zenyd commented Mar 6, 2021

it doesn't get wrapped to a new line when it's too long. Doesn't matter if vertical or horizontal.
Not much we can do. One can still view the whole list on the terminal.

For osd maybe output in blocks of two or three:

<file1>; <file2>; <file3>;
<file4>; <file5>; <file6>;
....

@bitingsock
Copy link
Contributor Author

It wraps for me:
Untitled
Untitled2

maybe you have a sub-ass-override somewhere?

@zenyd
Copy link
Owner

zenyd commented Mar 6, 2021

indeed I had sub-ass-override=force but even with that disabled it didn't wrap for me. Maybe it's ome other sub-* option?
I tried your changes with image files. Maybe it doesn't work there? Can you please check?

@bitingsock
Copy link
Contributor Author

gradient2

@bitingsock
Copy link
Contributor Author

I even tried it with no-config to make sure. still works

if timer is local showList() can't reference it
@bitingsock
Copy link
Contributor Author

couple other changes

@bitingsock
Copy link
Contributor Author

see if you have any osd-* in you config that might be different

@zenyd
Copy link
Owner

zenyd commented Mar 7, 2021

I tried with --no-config as well, but still the same. Anyway not so important.
Do you want me to merge?

@bitingsock
Copy link
Contributor Author

I'd like to at least figure the source of the problem even if we can't fix it. I'm on Win10 using Shinchiro's build. What are you using?

@zenyd
Copy link
Owner

zenyd commented Mar 7, 2021

I'm also using Win10 + Shinchiro's build. My build is a little dated though.
0.33.0-67-g8121d958ec
built on Sun Jan 31

@bitingsock
Copy link
Contributor Author

bitingsock commented Mar 7, 2021

Just tried that build. Works for me. That's pretty strange. Go ahead and merge, I guess.

@zenyd
Copy link
Owner

zenyd commented Mar 7, 2021

Could you document the new keybinds in the readme? With that done I can merge.

@zenyd
Copy link
Owner

zenyd commented Mar 7, 2021

found the issue why it doesn't wrap for me. It wrapped for you because you had spaces in your filenames. When you don't have spaces it doesnt break the line. I only tested with files that had no space. Adding a space after the semicolon fixed it for me.

Then again if the filename is too long, it get's clipped on the window border still and this we can't change I think.

@bitingsock
Copy link
Contributor Author

bitingsock commented Mar 7, 2021

Oh duh. I could fix some edge cases there like converting "." into spaces.

@bitingsock
Copy link
Contributor Author

but there should probably be a space after semicolon anyway

@zenyd
Copy link
Owner

zenyd commented Mar 7, 2021

one other thing I noticed.

When viewing the osd it still shows part of the path test/testfile.png;
On the terminal it show like this: [delete_file] deleting: C:\<...>\Desktop\test/testfile.png notice the /.

@bitingsock
Copy link
Contributor Author

bitingsock commented Mar 7, 2021

Ya I should do that differently. One sec

@bitingsock
Copy link
Contributor Author

K try that out

@zenyd
Copy link
Owner

zenyd commented Mar 7, 2021

works fine. Merging.

@zenyd zenyd merged commit efa300f into zenyd:master Mar 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants