Skip to content

Refresh Preferences documentation#198

Merged
kitchoi merged 3 commits into
masterfrom
doc-update-preferences
Nov 11, 2020
Merged

Refresh Preferences documentation#198
kitchoi merged 3 commits into
masterfrom
doc-update-preferences

Conversation

@kitchoi
Copy link
Copy Markdown
Contributor

@kitchoi kitchoi commented Nov 10, 2020

Closes #42

This PR updates the Preferences section in the documentation.

I re-run the code mentioned in the documentation and updated the output.

I also removed a block which advertises mutating the class attribute (which has a trait type) directly. If it had worked before, it does not work any more. I am tempted to say this is probably not a good pattern anyway to mutate global state on PreferencesHelper so I did not open a new issue.

There are some minor, opportunistic visual updates in the sections/subsections organization.
For example, a couple of sections are at the same level as "Preferences" itself, so previously the TOC looks confusing:
Screenshot 2020-11-10 at 15 54 47

Now those sections are made subsections, so the TOC looks like this:
Screenshot 2020-11-10 at 16 20 27

Checklist

  • Add a news fragment if this PR is news-worthy for end users. (see docs/releases/README.rst)

@kitchoi
Copy link
Copy Markdown
Contributor Author

kitchoi commented Nov 10, 2020

#42 is intended to document changes related to the difference between unicode and str, but since we are dropping Python 2, turns out there is no much we need to do regarding unicode. References to Unicode have been removed in #123 and #192

""" A preferences helper for the splash screen. """

PREFERENCES_PATH = 'acme.ui'
preferences_path = 'acme.ui'
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The use of PREFERENCES_PATH results in this warning message:

logger.warn('DEPRECATED: use "preferences_path" %s' % self)

node and from then on in, you don't have to pass a preferences collection in
each time you create a helper::

>>> PreferencesHelper.preferences = Preferences(filename='example.ini')
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This mutates class attribute (global state) and it also does not work... helper.bgcolor returns "", helper.width returns 0 (default of Int), etc.

Node() {}
Node(application) {}
Node(acme) {}
Node(ui) {'bgcolor': 'blue', 'width': '50', 'ratio': '1.0', 'visible': 'True'}
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed because since Python 3.7 dictionary order is preserved to match insertion order. So the output order is going to match the order in the preference file.

@@ -1,3 +1,5 @@
.. _preferences-in-envisage:

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Orthogonal... this section looks like it belongs to Envisage? Maybe a separate issue here.

Copy link
Copy Markdown
Contributor

@aaronayres35 aaronayres35 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes LGTM! I am yet to build the docs though

@aaronayres35
Copy link
Copy Markdown
Contributor

Changes LGTM! I am yet to build the docs though

just built the docs and still LGTM

@kitchoi
Copy link
Copy Markdown
Contributor Author

kitchoi commented Nov 11, 2020

Thanks. I will open the issue about the suspicious "Preferences in Envisage" section.

@kitchoi kitchoi merged commit addd26b into master Nov 11, 2020
@kitchoi kitchoi deleted the doc-update-preferences branch November 11, 2020 13:53
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.

update the documentation about Preferences

2 participants