Skip to content

Fix display of about any image please #3

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

Open
Pokefinder-org opened this issue Nov 17, 2023 · 5 comments
Open

Fix display of about any image please #3

Pokefinder-org opened this issue Nov 17, 2023 · 5 comments

Comments

@Pokefinder-org
Copy link

I tried several d64/d81/d71 images and MANY MANY are shown as empty or loose files compared to a c1541 output.
In this version this virtual filesystem cannot be seen as usable in ANY way - definately.

May sound like a biased rant but you are free to try. Packaging this with midnight commander should be be forbidden and punished.

After putting the latest checkout to /usr/lib/mc/extfs.d/uc1541 (not to forget "apt install python-is-python3" on an uptodate Debian 12 system) opening

http://ftp.pokefinder.org/tmp/wizball.d64

shows no files.

Logging:
2023-11-17 22:18:06,040 DEBUG 621 - Script params: ['/usr/lib/mc/extfs.d/uc1541', 'list', '/Data/torr/wizball.d64']
2023-11-17 22:18:06,049 DEBUG 157 _go_to_next_sector - Going to the track: 18, 1
2023-11-17 22:18:06,049 DEBUG 183 _go_to_next_sector - Next track: 0,255
2023-11-17 22:18:06,049 DEBUG 138 _map_filename - string: b'WIZBALL+12CH/REM''' mapped to: wizball+12ch/rem''
2023-11-17 22:18:06,049 DEBUG 138 _map_filename - string: b'WIZBALL HIGH/REM''' mapped to: wizball high/rem''
2023-11-17 22:18:06,049 DEBUG 153 _go_to_next_sector - End of directory
2023-11-17 22:18:06,049 INFO 379 list - List contents of /Data/torr/wizball.d64
2023-11-17 22:18:06,050 DEBUG 543 _call_command - executing command: c1541 -attach /Data/torr/wizball.d64 -list

Whats wrong here? Even though I find an attempt at replacing "/" with a PIPE VERY wrong, it doesnt even seem to happen here.

Some quick looks at d81 images show "lost files" as well. Shrug? WHY? Is a single made up test.d6z really enough for valid testing and actually shipping to many users?

Really - SO many images show up as wrong - its horrible.

I may be super-biased as I am related to the VICE project but this is in no way better than the old bash stuff.

@gryf
Copy link
Owner

gryf commented Nov 20, 2023

Thanks for the report. Turns out in Python 3.7+ there were changes regarding re module, which affects line matcher for the directory entries. Commit bda541d is fixing this. Apologies for inconvenience.

@Pokefinder-org
Copy link
Author

Many thanks for the quick fix. Looks ok at first glance - will test a few hard images and all operations and report back then. Never seen "/" being replaced by PIPE. Not sure thats a good idea but have to find a case where this hurts still :)
Underscore would be better IMHO - as said - testing.

@gryf
Copy link
Owner

gryf commented Nov 21, 2023

The mentioned wizball image have filenames contain / which will be replaced by |. Underscore is used for exchange of leading - character in the filename, as MC doesn't behave predictable when copying things into images. Perhaps it's a good time to revisit this, if anything changed.

@gryf
Copy link
Owner

gryf commented Nov 22, 2023

I've played around with the corner cases, and turns out it is not MC fault in this case. The biggest issue here is c1541 doesn't accept arguments for commands (filename in this case) which starts with hyphen:

$ c1541 -attach disk.d64 -write '- some file'
wrong number of arguments
syntax: write <source> [<destination>]

and that's no matter what escaping trickery is used, single/double quote, result is always failure, BUT, when writing such file from interactive session it works just fine:

$ c1541
c1541 (VICE 3.7.1)
Copyright 1995-2022 The VICE Development Team.
C1541 is free software, covered by the GNU General Public License, and you are
welcome to change it and/or distribute copies of it under certain conditions.
c1541 #8> attach disk.d64
c1541 #8> write "- some file"
writing file `- SOME FILE' as `- SOME FILE' to unit 8
c1541 #8> 

I didn't read the code for the c1541 implementation, but seems to me, that consuming commandline arguments isn't very well done.

Besides that, c1541 is really annoying to work with, as it doesn't have coherent way for displaying contents of the directory (CBM control characters are treated in ambiguous way - sometimes they are interpreted as a dot, sometimes as newline/carriage return/line feed, there is no way to get the "raw" directory listing - hence my pure python implementation for reading it out of the image), and finally there is no single way for communicating an error - sometimes stderr is used, sometimes stdout (sic!), and return code in case of an error is sometimes different than 0 while in some other cases not.

@Pokefinder-org
Copy link
Author

For the filename starting with "-" a bugfix was just commited to the vice tree which should fix issues.

please test
revision 44781 - /trunk/vice/src/c1541.c: when parsing cmdline, allow arguments/filenames starting with - whenever possible

c1541 -format "foo,00" d64 test.d64 -write "- test"
and
c1541 -format "foo,00" d64 test.d64 -write "- test" "- foo"
should work now but not
c1541 -format "foo,00" d64 test.d64 -write "- test" "-list" :)

will test on my side as well. Prolly close this issue and let me open a new one for major fails and problems.
About the error codes you could pass me some tests/details or an issue and the vice team will get annoyed by me.

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

No branches or pull requests

2 participants