Fixes for AmigaOS 4

Bug #1238689 reported by Chris Young
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
qpdfview
Fix Released
Wishlist
Adam Reichold

Bug Description

The attached file is a diff against v0.4.4 of qpdfview. It fixes some issues when qpdfview is built for AmigaOS 4:
1. Files specified on the command line have to be in UNIX path format (with this patch proper AmigaOS paths are accepted)
2. WB extended selection support added
3. Set the stack sufficiently high to stop it running out

Related branches

Revision history for this message
Chris Young (chris-0zxv) wrote :
Revision history for this message
Adam Reichold (adamreichold) wrote :

Hello Chris,

thank you for your contribution! It's nice to know that qpdfview is useful on non-Unix platforms. I am willing to include these modifications upstream, but for that I think that I have to understand them. :-)

First question is what the problem with "QFileInfo(...).absoluteFilePath();" is? Also this is as far I currently remember only necessary if the file path is passed on via D-Bus (since the called upon instance might have a different working directory), so we could maybe just move this into the D-Bus code path which would simplify the patch. Is D-Bus used on AmigasOS 4?

Best regards, Adam.

Changed in qpdfview:
assignee: nobody → Adam Reichold (adamreichold)
importance: Undecided → Wishlist
milestone: none → 0.4.7
status: New → In Progress
Revision history for this message
Adam Reichold (adamreichold) wrote :

Hello again,

the linked branch contains a form of the patch with which I would be comfortable carrying it upstream. Could you have a look at it and give it try on AmigaOS? Thanks.

Best regards, Adam.

Revision history for this message
Chris Young (chris-0zxv) wrote :

Hi Adam

The problem with absoluteFilePath is that our Qt libraries expect UNIX style file paths, and the function consequently translates an AmigaOS path which is already absolute into a nonsense UNIX style path, which Qt then can't find. For relative paths it works fine. I'm told this can't be fixed within Qt, and as you've noticed unless the current working directory is different (which it can't be in this case) absolute paths aren't needed anyway.

D-Bus isn't used on OS4, so if moving this there works for you that's probably the best solution.

Revision history for this message
Adam Reichold (adamreichold) wrote :

I see, thanks for explaining it. So it should probably be tucked in the D-Bus code path as in the linked branch. If that one works for you on AmigaOS, I'll merge it into trunk after 0.4.6 is released the day after tomorrow and it will be part of version 0.4.7.

Revision history for this message
Chris Young (chris-0zxv) wrote :

Nearly there, but the extended selection part isn't working (which is needed so double-clicking a PDF will open in qpdfview, so it's quite important) - ie. the section with the "started from Workbench" comment.

I put it at the top when I initially added it, because something manipulates the standard C argc/argv variables, which stops this from executing. IIRC it needs to be before QApplication::arguments().

Revision history for this message
Adam Reichold (adamreichold) wrote : Re: [Bug 1238689] Re: Fixes for AmigaOS 4

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Ok, I moved it before the regular command-line argument parsing and
hence before the call to "QApplication::arguments". If you give it one
more try so we know it does what it's supposed to? Thanks.

Am 11.10.2013 23:30, schrieb Chris Young:
> Nearly there, but the extended selection part isn't working (which
> is needed so double-clicking a PDF will open in qpdfview, so it's
> quite important) - ie. the section with the "started from
> Workbench" comment.
>
> I put it at the top when I initially added it, because something
> manipulates the standard C argc/argv variables, which stops this
> from executing. IIRC it needs to be before
> QApplication::arguments().
>
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)

iQEcBAEBAgAGBQJSWPSXAAoJEPSSjE3STU34mGkH/3G6f/a/vuibAo8Lf4Wttfne
LtMsrbC9Kq9N1pMY1hsi0tX9OJAdiz2pCUShfaikkBQFFIe7LjwxFkDLMQGBqi7p
B0XGzlh4pw0UgEa+Pnc9KLYHtFe0Jmw/DpDrVABggKdq7JZJ1pCEhBI+8hr9sO7F
wlBYk0RcXbPY3VQ0UphoXSoiR4VRxPPyDRtPX4hW7/s9pVungNj63+Qf6As9Kf9F
oB/FuUaOhYa15yVxIpzNe/3qj1y6g1I4kQopQ2FpAwRqRm9tXnZuxplmA4BAUnQH
0EbnsrdTcacIA2eYXL4XIBulQNrrGYUXCJhJ8DvOHmFC4H1fydEyIw9qQQrDEsw=
=Fm8u
-----END PGP SIGNATURE-----

Revision history for this message
Chris Young (chris-0zxv) wrote :

Sorry, I was mistaken, it's Application(argc,argv) which upsets it (it somehow sets argc to 1, I didn't check what it did to argv), so it will need to be above that.

Revision history for this message
Adam Reichold (adamreichold) wrote :

Ok, next try, thanks for bearing with me here. I moved it before constructing QApplication but I store the extended selection in a separate list which is then merged with the usual files list at the regular processing of the command-line arguments. Please check whether this handles both cases of argument processing for you. Thanks.

Revision history for this message
Chris Young (chris-0zxv) wrote :

Perfect, thanks!

Revision history for this message
Adam Reichold (adamreichold) wrote :

All right, we're ready to merge to trunk then which should happen tomorrow.

Changed in qpdfview:
status: In Progress → Fix Committed
Revision history for this message
Adam Reichold (adamreichold) wrote :

Hello Chris,

I did some refactoring of the main routine as I wasn't comfortable with carrying my own spaghetti code upstream any more. ;-) This also affects the WB extended selection part and if have a few minutes to spare, it would be nice if you could build a current snapshot from trunk to validate that I didn't break anything for AmigaOS.

Best regards, Adam.

Revision history for this message
Chris Young (chris-0zxv) wrote :

Hi Adam

Yes, it is still working as intended.

Thanks
Chris

Changed in qpdfview:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.