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

Doc: Revise section on info_plist for plistlib functionality. #3541

Merged
merged 6 commits into from Jun 8, 2018

Conversation

Projects
None yet
2 participants
@itsayellow
Contributor

itsayellow commented May 28, 2018

See Issue #3540
As plistlib has been added to the code to process the info_plist build argument dict, info_plist can now accept nested python objects, and python datatypes other than strings. This update to the documentation reflects these new capabilities and includes new examples, as well as removing descriptions of limitations that no longer exist.

Doc: Revise section on info_plist for plistlib functionality.
As plistlib has been added to the code to process the
info_plist build argument dict, info_plist can now
accept nested python objects, and python datatypes
other than strings.  This update to the documentation
reflects these new capabilities and includes new examples,
as well as removing descriptions of limitations that no
longer exist.
@htgoebel

That's been quick. Thanks :-)

XML.) The following python data types are translated to the proper
:file:`Info.plist` types: strings, integers, floats, booleans, tuples, lists,
dictionaries (but only with string keys), Data, bytes, bytesarray, and
datetime.datetime objects.

This comment has been minimized.

@htgoebel

htgoebel May 29, 2018

Member

IMO it would be okay to only refer to plistlib for details.

and follow it with a statement such as::
cp -f Info.plist dist/myscript.app/Contents/Info.plist
app = BUNDLE(exe,

This comment has been minimized.

@htgoebel

htgoebel May 29, 2018

Member

I suggest having only one example, which I asumme to be enough. So the order would be something like

  • old lines up to 373 ("...parameter to the BUNDLE call")
  • our new lines 390-393
  • this large example
  • short explanation (based on old lines 378-379 and your new lines 398-407)
'LSItemContentTypes': [
'com.example.myformat'
]
'LSHandlerRank': 'Owner'

This comment has been minimized.

@htgoebel

htgoebel May 29, 2018

Member

Trailing commas (separators) are missing at most of the lines above.

'CFBundleTypeIconFile': 'MyFileIcon.icns'
'LSItemContentTypes': [
'com.example.myformat'
]

This comment has been minimized.

@htgoebel

htgoebel May 29, 2018

Member

Use only one line for 'LSItemContentTypes': ['com.example.myformat'],?

@@ -383,36 +383,47 @@ also work in retina screen::
icon=None,
bundle_identifier=None
info_plist={
'NSHighResolutionCapable': 'True'
'NSHighResolutionCapable': True

This comment has been minimized.

@htgoebel

htgoebel May 29, 2018

Member

Please mind the same error in line 378

@itsayellow

This comment has been minimized.

Contributor

itsayellow commented May 29, 2018

Sure thing! I appreciate good documentation and was about to volunteer if you didn't ask. :)

For the edits, I tried to do as requested. (doh! Sorry about the missing commas.) Let me know how that looks.

@htgoebel

Thanks for the quick update. Looks goo, beside some typos.

|PyInstaller| creates :file:`Info.plist` from the info_plist dict
using the Python Standard Library module plistlib_.
plistlib can handle nested python objects (which are translated to nested
XML.) Python data types are translated to the proper :file:`Info.plist`

This comment has been minimized.

@htgoebel

htgoebel May 30, 2018

Member

"XML)." (dot behind parent)
or even
"XML), where …"

Also in the above example is the key ``CFBundleDocumentTypes``.
This entry in :file:`Info.plist` tells Mac OS X
what filetypes your app supports. (see `Apple document types`_).

This comment has been minimized.

@htgoebel

htgoebel May 30, 2018

Member

dot and space after "supports".

and follow it with a statement such as::
cp -f Info.plist dist/myscript.app/Contents/Info.plist
For example, when you use PyQt5,

This comment has been minimized.

@htgoebel

htgoebel May 30, 2018

Member

Is NSHighResolutionCapable related to PyQt5? If not, I'd remove the "when you use PyQt5,".

This comment has been minimized.

@itsayellow

itsayellow May 30, 2018

Contributor

This was existing in the documentation, and I didn't understand it either so I left it. To tell you the truth I've never used NSHighResolutionCapable. Usually the Info.plist key that's crucial for an application to have retina resolution enabled is 'NSPrincipleClass': 'NSApplication'.

This comment has been minimized.

@itsayellow

itsayellow May 30, 2018

Contributor

https://developer.apple.com/library/content/documentation/General/Reference/InfoPlistKeyReference/Articles/CocoaKeys.html#//apple_ref/doc/uid/TP40009251-SW11
https://developer.apple.com/library/content/documentation/GraphicsAnimation/Conceptual/HighResolutionOSX/Explained/Explained.html

Based on those two links, I'm starting to think that NSHighResolutionCapable is a red herring and should be removed from the example.

It seems that the crucial thing to enable retina is to inform macOS that the application is a Cocoa app, using 'NSPrincipleClass': 'NSApplication'. Then, the default value of NSHighResolutionCapable should already be True

The value of that key in :file:`Info.plist` is a list of dicts,
each containing up to five key:value pairs. This is represented in the
info_plist argument by assigning to the key ``CFBundleDocumentTypes`` a value
that is a python list containing python dicts.

This comment has been minimized.

@htgoebel

htgoebel May 30, 2018

Member

I do not understand this two sentences. Maybe just remove them?

This comment has been minimized.

@itsayellow

itsayellow May 30, 2018

Contributor

Yes, I was trying to use what was already there, but quite right--this got confusing. Removing them.

itsayellow added some commits May 30, 2018

@itsayellow

This comment has been minimized.

Contributor

itsayellow commented May 30, 2018

I've fixed the punctuation goofs (and rephrased one, let me know how it looks).

What do you think about removing NSHighResolutionCapable from example and discussion, and instead putting 'NSPrincipleClass': 'NSApplication'? (See discussion above.)

@htgoebel

This comment has been minimized.

Member

htgoebel commented Jun 3, 2018

Thanks for the update. I don't like red herrings :-)

Still I would like to have any other example for a boolean entry to show this has to be a boolean (and not a string or number). I would not care about using some pure example like aBooleanValue: True. If you know a real-life example, we could of course use it.

Add new boolean Info.plist example and 'NSPrincipleClass'.
Also clean up explanation paragraph after example.
@itsayellow

This comment has been minimized.

Contributor

itsayellow commented Jun 4, 2018

I agree, although my father loved (actual) red herrings...

I updated the example with a common boolean key/value. I added 'NSPrincipleClass'. Finally I cleaned up the explanation of the example.

Let me know how it looks!

@itsayellow

This comment has been minimized.

Contributor

itsayellow commented Jun 8, 2018

BTW, I will be unplugged and away on vacation starting Saturday morning. I'm happy if you want to make any changes in this pull request.

@htgoebel htgoebel merged commit 32a1bf0 into pyinstaller:develop Jun 8, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@htgoebel

This comment has been minimized.

Member

htgoebel commented Jun 8, 2018

Thanks for nagging :-)

Thanks you very much for this pull-request. I appreciated the discussion and really like the result. Esp. your explanation for the boolean example is even more elaborated than I your have expected it it be.

Thanks again ans I'm looking forward for more contributions from you. Maybe you are interested in working on other OS X related issues. Envoy your holidays.

@itsayellow

This comment has been minimized.

Contributor

itsayellow commented Jun 8, 2018

Thanks for the good words, and I was happy to contribute to such a friendly project! I will definitely take a look at the OS X issues (I already did a little bit.)

@itsayellow itsayellow deleted the itsayellow:info_plist_doc_update branch Jun 8, 2018

@htgoebel htgoebel added this to the PyInstaller 3.4 milestone Sep 2, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment