Add-on packaging #213

Closed
nvaccessAuto opened this Issue Jan 1, 2010 · 63 comments

1 participant

@nvaccessAuto

Reported by aleksey_s on 2008-10-27 14:38
as nvda goes much power many optional components might be added. for now we have only brltty and the newfon synth, further anything which will comes on. so i think we need to make some exact way for users to manage nvda components. it might be implemented with help of nsis or a standalone program. from user's side this will be a list of optional components which he can install or remove. currently, these might be followed components:

  • espeak russian additional data
  • espeak chinese additional data
  • liblouis library and tables (one might want not have it, becouse he/she haven't braille display)
  • brltty package
  • newfon synthesizer
  • stress dictionary for newfon synthesizer
  • sapi 4 and 5 runtimes

from developers side, it must be easy manageable list of packages. so, one database accessible from the website with some fields and packages theirselves. based on program with which to implement this feature, packages might to be in different formats. i don't know nsis features, but it may be script to run for optain package. it might just download and run executable (sapi runtime installation), or download, unzip and copy some files into nvda folder. each package might have different license agreement, which user must to accept.

@nvaccessAuto

Comment 1 by jteh on 2008-11-02 20:09

  • Is this really necessary? Why not just self-extracting archives or installers?
    • These are pretty simple, although they do require some knowledge of where NVDA is installed.
    • I still don't think the BRLTTY installer should ever be integrated as an NVDA component. It is too complex and is very much a separate package.
    • The Newfon synth is probably a good candidate for this.
  • Putting this into the installer means that the user has to reinstall NVDA to install these components. Also, some users may not use the installer. I think this would be better done as a stand alone program.
  • I don't see the need for a client side downloader or package database.
    • Why not just a simple web page from which users can download packages? The web is already a very familiar environment.
    • This also makes maintenance very simple.
    • Other community members can potentially create their own packages.
    • The NVDA portion can be a simple package installer which basically just extracts the package.
    • Do we need an uninstaller? This requires tracking of package data, but this probably isn't too hard.
    • How will the NSIS installer deal with this? It will leave package files behind. How do we indicate that these must be uninstalled?
@nvaccessAuto

Comment 2 by Bernd on 2009-02-22 02:45
I think such a page, where users can upload and download there packages will be quite good. Should I open an extra Ticket? If yes, I think we could close this ticket as wontfix.

@nvaccessAuto

Comment 3 by jteh on 2011-12-13 07:04
Narrowing the scope of this ticket to just cover the packaging part, rather than the repository. For now, the latter can be handled simply using a wiki page on the NVDA web site.

We need add-on packages because many users aren't comfortable with extracting files into specific locations and it is often difficult to explain where files should be extracted. My current thinking is outlined below.

Package format:

  • Packages would be a zip archive with a defined naming convention; e.g. blah.nvdaAddon.zip, nvdaAddon_blah.zip, or blah.nvdaAddon.
  • There would be a small text file in the root dir of the archive providing metadata for the add-on, including name, version, publisher, minimum/maximum compatible NVDA version and description.
  • The archive would also contain folders containing Python modules for appModules, globalPlugins, synthDrivers and brailleDisplayDrivers as appropriate.

Installation:

  • NVDA will provide a menu item to install an add-on package.
  • When activated, the user will be asked to locate and choose the add-on package file.
  • Assuming all checks are okay, the user will be prompted for confirmation to install. Information about the package will be provided here.

Management:

  • A dialog will be provided to manage packages, including uninstallation.
  • Updates could be handled here too, although I think this should be implemented later if we implement it at all.

Misc technical:

  • The package files will be placed in userConfig\addons\packageName.
  • Subdirectories of these directories will be added to the appModules, globalPlugins, etc. Python package paths as appropriate. Changes: Changed title from "possibly task: something like the nvda's optional components repository" to "Add-on packaging" Milestone changed from None to 2012.1
@nvaccessAuto

Comment 4 by aleksey_s (in reply to comment 3) on 2011-12-13 18:47
quick thoughts:
Replying to jteh:

Narrowing the scope of this ticket to just cover the packaging part, rather than the repository. For now, the latter can be handled simply using a wiki page on the NVDA web site.

may be a wiki page of specific format, to be easily programatically parsed for automatic discovering.

  • Packages would be a zip archive with a defined naming convention; e.g. blah.nvdaAddon.zip, nvdaAddon_blah.zip, or blah.nvdaAddon.

the later can be useful for automatic NVDA file association. Installer should associate .NVDAAddon file extension with NVDA for convenience.

  • There would be a small text file in the root dir of the archive providing metadata for the add-on, including name, version, publisher, minimum/maximum compatible NVDA version and description.

Description should be localizable. maybe it is worth making metadata inside the extension python code. This way, if developer does cary about localization, he has a way to return localized strings.

  • The archive would also contain folders containing Python modules for appModules, globalPlugins, synthDrivers and brailleDisplayDrivers as appropriate.

as well as other addon specific files (configuration, localization etc).

  • The package files will be placed in userConfig\addons\packageName.

  • Subdirectories of these directories will be added to the appModules, globalPlugins, etc. Python package paths as appropriate.

Anyway, we need some sort of registry of installed addons. therefore, we can store list of files related to each addon. So, is it worth creating a separate dir for each addon, considering that almost 100% of those will be only one thing (app module, synth, braille display driver or global plugin)?

@nvaccessAuto

Comment 5 by ragb (in reply to comment 3) on 2011-12-19 19:07
My thoughts on this, as it is something I find important:
[to comment:3 jteh:

Narrowing the scope of this ticket to just cover the packaging part, rather than the repository. For now, the latter can be handled simply using a wiki page on the NVDA web site.

We need add-on packages because many users aren't comfortable with extracting files into specific locations and it is often difficult to explain where files should be extracted. My current thinking is outlined below.

+1

Package format:

  • Packages would be a zip archive with a defined naming convention; e.g. blah.nvdaAddon.zip, nvdaAddon_blah.zip, or blah.nvdaAddon.

When I read this the first thing that came to mind was python egg files:
http://mrtopf.de/blog/en/a-small-introduction-to-python-eggs/

Those are much coppled with setuptools and such but, if things are done right in those libraries, maybe stuff can be reused. More on that billow.

  • There would be a small text file in the root dir of the archive providing metadata for the add-on, including name, version, publisher, minimum/maximum compatible NVDA version and description.

Do you think that something similar a setup.py won't work? I can see some security problems but the same already applies for any plugin that can execute python code. That would possibly solve the description localization issue pointed by alexei in another comment.

  • The archive would also contain folders containing Python modules for appModules, globalPlugins, synthDrivers and brailleDisplayDrivers as appropriate.

One can add those to python path using pkg_resources machinery quite easily. Actually I think that it is even not necessary to extract the zip file (assuming that will be a zip file) for many packages since python can import from zip files. However that can possibly create problems with loading of not-python data such as dlls, sounds, etc.

Installation:

  • NVDA will provide a menu item to install an add-on package.

  • When activated, the user will be asked to locate and choose the add-on package file.

  • Assuming all checks are okay, the user will be prompted for confirmation to install. Information about the package will be provided here.

Seems good.

Management:

  • A dialog will be provided to manage packages, including uninstallation.

  • Updates could be handled here too, although I think this should be implemented later if we implement it at all.

When I thought about setuptools/distutils/distribute it was also for this purpose. It is very reasonable that some add-on would depend on another one so updating and dependency tracking could be eventually needed. Because one does not need to reenvent things when they exist (unless they don't exist) I think that those might be a starting point.

Misc technical:

  • The package files will be placed in userConfig\addons\packageName.

  • Subdirectories of these directories will be added to the appModules, globalPlugins, etc. Python package paths as appropriate.

Ok, the pkg_resources mentioned above.

Regards,

Rui Batista

@nvaccessAuto

Comment 6 by ragb on 2011-12-28 22:39
Reading trhough this again:

In my previous comment when it reads pkg_resources it should be pkgutils. pkg_resources another different thing.

Anyway, I've been reading some parts of NVDA source code (to update myself on things) and in my opinio, with the current state of the code, this packaging thing is not as hard as I thought before.

If needed I can work on this since I have to do something else than my master thesis to not get mad... I don't think I have accessibility related skills to help with more iaccessible and friends related stuff but on this field I can colaborate. So, if you want I can try to implement this packaging thing, or at least some parts of it.

@nvaccessAuto

Comment 7 by jteh (in reply to comment 4) on 2011-12-28 23:09
Replying to aleksey_s:

the later can be useful for automatic NVDA file association. Installer should associate .NVDAAddon file extension with NVDA for convenience.

This isn't possible for portable users, so I was a bit reluctant to do something special for the installer, as it makes documentation tricky; i.e. you have to document one method for the installer and one method for the portable. However, I admit it would be simpler this way. Also, I'd prefer an extension like nvdaAddon.zip so that it's clear it's a zip archive and developers don't have to rename it, but Windows extension association may not handle this.

Description should be localizable. maybe it is worth making metadata inside the extension python code.

I thought of doing this with Python code. It would be simpler for a lot of reasons. However, it presents a major security problem. The module could contain harmful code which would be executed before the user had even confirmed installation of the package, which is very bad. Perhaps the metadata file will have to contain text for each locale. Ugly.

Anyway, we need some sort of registry of installed addons. therefore, we can store list of files related to each addon. So, is it worth creating a separate dir for each addon

That's what I said: userConfig\addons\packageName. App modules would be in userConfig\addons\packageName\appModules, etc.

@nvaccessAuto

Comment 8 by jteh (in reply to comment 5) on 2011-12-28 23:18
Replying to ragb:

When I read this the first thing that came to mind was python egg files:

Those are much coppled with setuptools and such but, if things are done right in those libraries, maybe stuff can be reused. More on that billow.

I'm reluctant to use setuptools because it contains a lot of extra stuff that we don't need, which would therefore unnecessarily increase the size of the NVDA distribution.

Do you think that something similar a setup.py won't work? I can see some security problems but the same already applies for any plugin that can execute python code.

That's true, but this will be loaded before the user even accepts installation of the package to display the name, publisher, description, etc. of the package. If the user accepts installation, they accept the possible security risk. However, loading code before the user even accepts this risk is unacceptable.

That would possibly solve the description localization issue pointed by alexei in another comment.

One can add those to python path using ![pkgutils] machinery quite easily.

We already have code to handle this. See config.addConfigDirsToPythonPackagePath. We'd need to modify this slightly.

Actually I think that it is even not necessary to extract the zip file (assuming that will be a zip file) for many packages since python can import from zip files. However that can possibly create problems with loading of not-python data such as dlls, sounds, etc.

I think it's best to always extract, as a lot of packages will depend on data which must be extracted.

When I thought about setuptools/distutils/distribute it was also for this purpose. It is very reasonable that some add-on would depend on another one so updating and dependency tracking could be eventually needed. Because one does not need to reenvent things when they exist (unless they don't exist) I think that those might be a starting point.

I'm happy for you to investigate this and report here. However, I am very concerned about unnecessary code in setuptools. Also, handling dependencies is problematic, as then you have to be concerned about automatic downloads, etc., not to mention the GUI for this. That said, I haven't actually investigated setuptools at all yet.

@nvaccessAuto

Comment 9 by ragb (in reply to comment 7) on 2011-12-29 18:15
Replying to jteh:

Replying to aleksey_s:

Description should be localizable. maybe it is worth making metadata inside the extension python code.

I thought of doing this with Python code. It would be simpler for a lot of reasons. However, it presents a major security problem. The module could contain harmful code which would be executed before the user had even confirmed installation of the package, which is very bad. Perhaps the metadata file will have to contain text for each locale. Ugly.

I guess that is the simplest option. NVDA installer already does something like this. Maybe this implies the usage of a more structured format for the plugin manifest data (say xml, json, etc). This can also be more extensible for the future than .ini files for instance.

@nvaccessAuto

Comment 10 by ragb (in reply to comment 8) on 2011-12-29 18:20
Replying to jteh:

Replying to ragb:

When I read this the first thing that came to mind was python egg files:

Those are much coppled with setuptools and such but, if things are done right in those libraries, maybe stuff can be reused. More on that billow.

I'm reluctant to use setuptools because it contains a lot of extra stuff that we don't need, which would therefore unnecessarily increase the size of the NVDA distribution.

That is really something to take in consideration.

Do you think that something similar a setup.py won't work? I can see some security problems but the same already applies for any plugin that can execute python code.

That's true, but this will be loaded before the user even accepts installation of the package to display the name, publisher, description, etc. of the package. If the user accepts installation, they accept the possible security risk. However, loading code before the user even accepts this risk is unacceptable.

Yes. Python code for the plugin data does not work.

One can add those to python path using ![pkgutils] machinery quite easily.

We already have code to handle this. See config.addConfigDirsToPythonPackagePath. We'd need to modify this slightly.

Yes, I checked that. That is great.

Actually I think that it is even not necessary to extract the zip file (assuming that will be a zip file) for many packages since python can import from zip files. However that can possibly create problems with loading of not-python data such as dlls, sounds, etc.

I think it's best to always extract, as a lot of packages will depend on data which must be extracted.

Yes, it esimplifies things alot. Python zipfile module has all it is needed, I think.

When I thought about setuptools/distutils/distribute it was also for this purpose. It is very reasonable that some add-on would depend on another one so updating and dependency tracking could be eventually needed. Because one does not need to reenvent things when they exist (unless they don't exist) I think that those might be a starting point.

I'm happy for you to investigate this and report here. However, I am very concerned about unnecessary code in setuptools. Also, handling dependencies is problematic, as then you have to be concerned about automatic downloads, etc., not to mention the GUI for this. That said, I haven't actually investigated setuptools at all yet.

Will do.

@nvaccessAuto

Comment 11 by jteh on 2012-01-05 08:04
I investigated setuptools a bit.

  • NVDA itself will only need pkg_resources, so bundling this isn't a problem.
  • Eggs do not support localisation of package metadata. I'm thinking we should just not bother with package metadata (name, description, etc.) at all in the actual packages, instead just relying on web sites to provide this info to users. Handling localised package metadata is proving to be way too painful.
  • Add-on developers will need to have Python and setuptools installed to build eggs. If it's just these two, I guess this isn't a problem. However, if a complete or even partial NVDA source environment is required, this isn't acceptable, as we don't want developers to have to run NVDA from source due to this being rather complicated.
  • The entry points support might actually work quite well for us. It would allow us to query for components without importing every add-on module and would eliminate the need for separate directories for each component type (appModules, globalPlugins, etc.). However, we'd probably still need to maintain the old system for backwards compatibility. I'm also not sure how to expose entry points for built-in plugins and drivers, as I suspect py2exe will not include our egg-info, so we may need to maintain the old system for those too.
  • The dependencies support looks okay, though I'm not sure whether you can check egg dependencies with pkg_resources before the egg is installed. Also, we need to somehow expose distribution info for NVDA itself (so that packages can require particular versions of NVDA) and I'm not really sure how to do this, again because I don't think py2exe will include our egg-info. Changes: Changed title from "Add-on packaging" to "EAdd-on packaging"
@nvaccessAuto

Comment 12 by jteh on 2012-01-05 08:05
I think NVDA's version numbering scheme is incompatible with the scheme required by Python, setuptools, etc., so this will cause problems if we want to use setuptools.

@nvaccessAuto

Comment 13 by jteh on 2012-01-05 08:06
Changes:
Changed title from "EAdd-on packaging" to "Add-on packaging"

@nvaccessAuto

Comment 14 by ragb (in reply to comment 12) on 2012-01-05 11:37
Replying to jteh:

I think NVDA's version numbering scheme is incompatible with the scheme required by Python, setuptools, etc., so this will cause problems if we want to use setuptools.

Is this also valid for stable releases or just snapshots from BZR? I can't get with anything in i.g. 2011.3 or 2012.1beta1 or any of the variations used that goes against the versions schema described in setuptools. With snapshots that is true, starting from the usage of dashes between thre brance's name and revision number.

@nvaccessAuto

Comment 15 by ragb on 2012-01-10 01:19
Reflecting further about this, I am more inclined to wards setuptools and egs. The y2exe problems may be solved later, eg-info may be included in the package and put on the eg path some how. To be sure I must really try it.

Regarding the versionnning schema, can't you change the snapshots versioning to comply with setuptools? It may provide mroe benefits later. With the repackaging code things will change in some ways so that could be an oportunity. Just an opinion though.

So, shall I start coding something in this setuptools / eg ppackages path?

@nvaccessAuto

Comment 16 by jteh (in reply to comment 15) on 2012-01-10 01:29
Replying to ragb:

Reflecting further about this, I am more inclined to wards setuptools and egs.

Any particular reason? I'm starting to think eggs are far more trouble than they're worth. We still have to mess about with modifying path on all packages unless we use entry points. Using entry points requires a lot of change which will either break backwards compatibility or make things confusing; e.g. different way of developing for internal versus add-ons.

Regarding the versionnning schema, can't you change the snapshots versioning to comply with setuptools?

This must take code running from source into account as well. Snapshot and source versions would end up being something like 2012.1a0.dev-bzr-main-4883, which is just hideous. It would probably change the output filename as well, which would require significant changes to the automatic build scripts. Branches other than main can be between releases (i.e. not yet 2012.2 but not 2012.1 either. Finally, 2012.1a0 is obviously considered to be before 2012.1, but this means that any add-ons for snapshots have to specify 2012.1a0, which is pretty ugly.

I'm also not convinced that dependencies are even a good idea. This is going to become incredibly complicated, as users will then expect that dependencies download automatically, etc. This must be handled when a package is removed as well.

@nvaccessAuto

Comment 17 by ragb (in reply to comment 16) on 2012-01-18 23:06
Sorry for the late response, real life got in the way..
Replying to jteh:

Replying to ragb:

Reflecting further about this, I am more inclined to wards setuptools and egs.

Any particular reason? I'm starting to think eggs are far more trouble than they're worth.

My rational is to got things more pythonic. Developpers used to python, setuptools, and easy_install might benefit from this kind of tools. However in the windows world things are not so straight-forward (I've been doing some python web development lately and in this case easy_install/distribute/... realy shine). NVDa itself is not packaed in that decoupled way, and honestly I don't think it would be worthwhile the effort to make it. yI believe ou have invested much time on the build infrastructure.

We still have to mess about with modifying path on all packages unless we use entry points. Using entry points requires a lot of change which will either break backwards compatibility or make things confusing; e.g. different way of developing for internal versus add-ons.

Yes. And changing to entry-points would break backwards compatibility. The current appmodules/brailleDisplayDrivers/... code must be kept arround. And the step curve for casual developers may be to high for casual developers (most appmodule developers are, so I believe).

Regarding the versionnning schema, can't you change the snapshots versioning to comply with setuptools?

This must take code running from source into account as well.

Hmmm. Ok. Forgot that.

Snapshot and source versions would end up being something like 2012.1a0.dev-bzr-main-4883, which is just hideous.

Many projects use that though... Sounds funy but not practical. And might confuse users used to current versioning schema. Snapshots are used by many people dispite beeing declared not stable. And user support nightmare would come...

It would probably change the output filename as well, which would require significant changes to the automatic build scripts. Branches other than main can be between releases (i.e. not yet 2012.2 but not 2012.1 either. Finally, 2012.1a0 is obviously considered to be before 2012.1, but this means that any add-ons for snapshots have to specify 2012.1a0, which is pretty ugly.

HMM Ok. Got it. Confusing...:)

I'm also not convinced that dependencies are even a good idea. This is going to become incredibly complicated, as users will then expect that dependencies download automatically, etc. This must be handled when a package is removed as well.

Ok. The main usecase is for stand-alone snapshots and may be bettter to stick to that for now.

@nvaccessAuto

Comment 18 by ragb on 2012-01-18 23:27
My ideas about the addon format, based on the discussion until now:

  • Addons are composed of python code, data (localization, dlls, etc) and a manifest file.
  • Python code is used to extend NVDA on specific extension points: new braille display drivers, application modules, global plugins and synth drivers. In the future more points can be added (I can't see one for now). We can stick to the pre-defined packages and class names approach used currently (i.g. GlobalPlugin class in a module contained in the globalPlugins package) or declare the extension points within the manifest file. What do you think would be the best solution?
  • The addon must be self-contained. That is, all data and python code needed must be contained on the module. Dependencies are not supported. Only standard library and other common python packages bundled in NVDA binaries may be used (what packages? Even with standard library modules we must document it carefuly since not all standard library is bundled).
  • The manifest file is an XML file (I'm open to other extensible formats such as yaml, json, etc) contains at least, addon name, version, description, author and url. Minimum NVDA compatible version is also important but I don't know if it should be mandatory or not. Moreover one must take in account snapshots and stable versions. Should all be considered or addons should require only compatiblity with oficial versions? Other extra information may be present. If extension points are explicitly declared (see above) this manifest would also contain that information. The Manifest file would be called something like manifest.xml or any other default name. A simple command line tool may be created to help with the creation of the manifest and the addon package in the required format.
  • The addon contents would be bundled in a zipfile with any dedicated extension (nvdaaddon). In the root there should be the manifest file and the addon python code and data. This zifile, on activation of the addon, would be extracted to something like userconfig/packages/. Note that the manifest can be read without extracting the package.

What do yout think?

@nvaccessAuto

Comment 19 by jteh on 2012-01-18 23:54
Changes:
Milestone changed from 2012.1 to 2012.2

@nvaccessAuto

Comment 20 by ragb on 2012-01-29 15:08
Regarding the manifest file format I think that config file sintax would work. I didn't too well the configobj library so I was a bit reticent to that. But as it support lists and such the extensibility needs I was thinking are well-covered. I'm prototyping some of these ideas right now. Will point for a branch when I find somewhere to host it (probably launchpad).

@nvaccessAuto

Comment 21 by ragb on 2012-01-30 23:56
Hi,

Here it is a preliminary specification of the manifest for an NVDA add-on. This only considers the add-on directory similar to what already exists on userconfig: pre-defined forlders and names for specific extensibility points.
This is in configobj configspec format, because it is what I'm using for prototyping the idea (see previous comments).

# NVDA Add-on Manifest configuration specification
# Add-on name
name = string()
# Quick description of the add-on to show to users.
description = string()
# Long description with further information and instructions
long_description = string(default=None)
# Name of the author or entity that created the add-on
author = string()
# Version of the add-on. Should preferably in some standard format such as x.y.z
version = string()
# URL for more information about the add-on. New versions and such.
url= string(default=None)
# Categories where this add-on belongs to. See list (to be defined)
categories = list(default=list())

# Compatibility with NVDA releases
# Only considering stable versions.
[Minimum version compatible with this add-on.
minVersion = string(default=None)
# Maximum compatible version.
maxVersion = string(default=None)

# Extra data to bundle with the add-on.
# Python files and packages are bundled by default.
# Must be relative to the base directory where are this manifest.
[data](compatibility]
#)
# Extra files to include. Can use glob expressions.
include = list(default=list())
# Directories to include recursively.
include_recursive = list(default=list())

This is a draft. I'm not considering compatibility with snapshots (how to do that?) nor explicit entry-points (something it is worth think about in my opinion) but as I said, this is just a prototype. Already implemented bundling of add-ons based on this manifest spec and I'm in the process of implementing extraction for userconfig/packages/ and patching NVDA to use code from those packages. Possible next steps may be, according to feedback:

  • User activation/deactivation of add-ons (user interface with list or something).
  • Patch nvda slave to install add-ons based on bundle extension (is .nvda-addon fine?).
  • Sure others.
@nvaccessAuto

Comment 22 by jteh on 2012-01-31 00:34
As per comment:11, I'm wondering whether we should just not bother with textual package metadata at all, as it creates a localisation issue that isn't easy to solve. Initially, the author would just provide information on whatever web site in their preferred language (English for the official web site, but other communities could I guess host package indexes as well). Eventually, there may be some sort of global package index which provides information in multiple languages.

I'm also not sure there's really a need for a manifest of files. I'd think any files in the archive would just be extracted and the manager would handle tracking of installed files.

@nvaccessAuto

Comment 23 by ragb (in reply to comment 22) on 2012-01-31 11:42
Replying to jteh:

As per comment:11, I'm wondering whether we should just not bother with textual package metadata at all, as it creates a localisation issue that isn't easy to solve. Initially, the author would just provide information on whatever web site in their preferred language (English for the official web site, but other communities could I guess host package indexes as well). Eventually, there may be some sort of global package index which provides information in multiple languages.

I understand your concerns about the localization issue, it surely requires additional work. ut eventualy we will have the need for a package index and users will ask for localization... If present in package metadata one can already use it for the NVDA interface and the index, keepping that information centralized. The first solution that comes to mind for this purpose is extending the metadata file with a localization section, like the folowing:

...
[= Descrição do meu addon
long_description = """ uma descrição mais detalhada
do meu add-on"""
[[es](i18n]
[[pt_pt]]
description)]
... # the same for spanish, I won't try :)...

This requires the translator to edit the metadata file or provide it to the author. But gettext integration can be investigated, as it is already done in intltool-merge on Linux. I think the first solution is easier to everyone though. In the meantime I think that, for an initial phase we could have this localization issue open but try to come with something working first, without thinking too much about localization.

Regarding the metadata itself (name, description, url...) I think it is really necessary so the user can get more information about the installed add-ons or some add-on he wants to install. Sadly most existing plugins that are hanging around are distributed on mailling-lists, twitter, drobox, etc. As we can not change that so easily at least plugin makers can provide some information without having users consult a website. This is just a rational. Compatible versions and such information must be provided somewhere though.

I'm also not sure there's really a need for a manifest of files. I'd think any files in the archive would just be extracted and the manager would handle tracking of installed files.

I'm not sure of any of the alternatives: bundle whatever tat is on the add-on directory or explicitly require the data to be bundled. Bundling everything creates the the situation where some less experienced people would bundle many stuff that is nod needed: hiden files, backup files (those ending with ~ or .swp), python source files (python bytecode alone works), gettext source translations, and so on. Not providing the source may be seen as bad, but add-on makers can delete .py files before zipping for the same purpose... Explicitly requiring metadata listing (python modules and packages ofcourse are implicit) puts more work on the hands of the add-on maker though.

@nvaccessAuto

Attachment addons-213-1.diff added by ragb on 2012-02-07 21:43
Description:
BZR bundle with first prototype of add-on integration.

@nvaccessAuto

Comment 24 by ragb on 2012-02-07 21:50
This patch is the first prototype at integrating add-ons.

For now addons are read from ./addons or userConfig/addons. Directories with a manifest.ini file (spec defined on AddonManifest class) are considered valid and loaded. If the add contains the usual extension directories (brailleDisplayDrivers, appModules,...) those are add to the modules python path. The problem is that for testing must be created by hand.
A sample manifest.ini for these purposes is as follows:

name = sampleAddon
description = My sample addon package
author = sample author
version = 0.1

In the mentioned addons directory create a sampleAddon directory and a manifest.ini file with those contents. Then put some plugin or something there to test.

Note that this is not done, I'm submitting for early reviewing.

@nvaccessAuto

Comment 25 by ragb on 2012-02-20 18:29
Hi,

I' m slowly making progress on this. Some features I thinkk are good to implement:

  • The user interface (NVDA menu) should be extendable by addons, to provide ways, for instance, to present addon specific configuration dialogs. One usecase: an addon contains a braille display driver for one display that uses the serial port (not currently supported without brltty). The addon developer can make a dialog for the user to choose the COM port. In my opinion an extras menu would be great, pus some function to add a menu entry and a callback for that entry. Obiously one can make a globalPlugin with a keybinding to open that dialog but some day key combinations are not enough.
  • To help with this (and more usecases) I purpose a general hooking system for addons. That means something like the following: in a hooks.py module present on the addon the develper some functions with pre-defined names like nvda_started, activate and deactvate (for addon activation and deactivation) and so on. nvda_started hook could be used for adding a extra menu entry, as described above. I already implemented this feature on my ocal branch
  • An option to run NVDA without addons would be great for debugging problems on user systems. With that one can be sure that a problem is not caused by some misbehaving addon.

I'm still developing on a local branch. I'm having issues with launchpad. Is it possible and easy for you to provide a branch on nvaccess servers?

@nvaccessAuto

Comment 26 by jteh (in reply to comment 25) on 2012-02-20 20:56
Replying to ragb:

I' m slowly making progress on this. Some features I thinkk are good to implement:

These are all interesting ideas, but they are outside the scope of this ticket. They should all be addressed in separate tickets (and therefore separate branches).

I'm still developing on a local branch. I'm having issues with launchpad. Is it possible and easy for you to provide a branch on nvaccess servers?

Can you be more specific about the issues you're having with Launchpad?

@nvaccessAuto

Comment 27 by ragb (in reply to comment 26) on 2012-02-20 21:50
Replying to jteh:

Replying to ragb:

I' m slowly making progress on this. Some features I thinkk are good to implement:

These are all interesting ideas, but they are outside the scope of this ticket. They should all be addressed in separate tickets (and therefore separate branches).

Ok. Will create those when applicabe.

I'm still developing on a local branch. I'm having issues with launchpad. Is it possible and easy for you to provide a branch on nvaccess servers?

Can you be more specific about the issues you're having with Launchpad?

I can't commit to launchad under windows. It's something related with my ssh publi key, but I never understood the problem. I didn't create a nvda specific branch yet, but it happened on other projects. Under linux everything goes well. Will try again shortly. Btw, the lp:nvda branch on launchpad is not beeing mirrored since 2011-07-13.

@nvaccessAuto

Comment 28 by jteh (in reply to comment 27) on 2012-02-20 21:57
Replying to ragb:

I can't commit to launchad under windows. It's something related with my ssh publi key, but I never understood the problem.

You will probably have the same problem on whatever server you use. I'd suggest further debugging on this. I'm happy to help via nvda-dev if you can provide error messages and the like.

Btw, the lp:nvda branch on launchpad is not beeing mirrored since 2011-07-13.

Damn. Thanks for reporting. I shall investigate.

@nvaccessAuto

Comment 29 by ragb (in reply to comment 28) on 2012-02-21 00:09
Replying to jteh:

Replying to ragb:

I can't commit to launchad under windows. It's something related with my ssh publi key, but I never understood the problem.

You will probably have the same problem on whatever server you use. I'd suggest further debugging on this. I'm happy to help via nvda-dev if you can provide error messages and the like.

Ok, problem solved. I lost my commit messages when merging (bzr seems to be a bit different from git) but have created a branch with my changes. Hopefully no more problems.

Branch is on

lp:~ruiandrebatista/nvda/addons-packaging

@nvaccessAuto

Comment 30 by ragb on 2012-02-21 19:41
Report of current situation, as per revision 5018 of lp:~ruiandrebatista/nvda/addons-packaging

Addons are directories with a similar structure to actual userConfig directory: they contain !brailleDisplayDrivers, !globalPlugins, etc. An add-on is installed to userConfig\packageName. When the addon files are installed the !brailleDisplayDrivers, !globalPlugins, etc... directories are added to the respective package paths, as it is already done for the userConfig directory. config.addConfigDirsToPythonPackagePath was slightly changed for this purpose.

Metadata for each addon is contained in \manifest.ini. This file is a common .ini file, loaded using configObj. The specification is as follows:

# NVDA Ad-on Manifest configuration specification
# Add-on name
name = string()
# Quick description of the add-on to show to users.
description = string()
# Long description with further information and instructions
long_description = string(default=None)
# Name of the author or entity that created the add-on
author = string()
# Version of the add-on. Should preferably in some standard format such as x.y.z
version = string()
# URL for more information about the add-on. New versions and such.
url= string(default=None)
# Categories where this add-on belongs to.
categories = list(default=list())

# Compatibility with NVDA releases
# Only considering stable versions.
[compatibility]
# Minimum version compatible with this add-on.
minVersion = string(default=None)
# Maximum compatible version.
maxVersion = string(default=None)

NVDA add-ons are distributed in packages called bundles wich are simple zipfiles with the add-on directory contents, including the manifest file. Addon bundle name must be in the form -.nvdaadon. Functions are provided to install and extract add-ons to userConfig/addons and to create a new add-on bundle from a directory. See below. For now all present add-ons are considered active but an interface must be provided for the user to manage all add-ons, install, activate, deactivate, and uninstall.

The !addonHandler.py module manages the addOns subsystem. Method initialize loads all available add-ons and activates them. terminate does the oposite. installAddonBundle receives a path to a addon bundle file and install and extracts its contents to userConfig/addons. createAddonBundleFromPath creates a add-on package file suitable for distribution from a directory that contains a manifest.ini file. Last two methods work fine from the NVDA Python console, while no interface is produced.

Some classes are worth mention: addonHandler.Addon represents an installed add-on. It has methods to activate, deactivate, get the manifest metadata and more. addonHandler.AddonBundle represents a bundle package (zipfile) and has methods to extract, retrieve manifest data without extracting, etc. addonHandler.AddonManifest is the manifest metatada, a simple subclass of configobj.ConfigObj.

In my opinion, the only thing that is missing for the complition of this task (not refering testing and such) is the itnerface to manage add-ons. All the other ideas are worth separate tickets. Do you agree?

@nvaccessAuto

Comment 31 by ragb on 2012-03-22 17:49
Should this be targeted for 2012.2??

@nvaccessAuto

Comment 32 by jteh on 2012-04-08 08:28
@ragb: Just so you know, I do plan to review your code, probably in the next few weeks. We've been very snowed under with other work for a while. :)

This is already scheduled for 2012.2.

@nvaccessAuto

Comment 33 by jteh on 2012-04-18 06:33
This code looks very good and functional. Very nice work.

Questions:

  • What do you imagine hooks being used for? One possibility I see is an installation hook; e.g. to gather data from the user, etc.

Problems:

  • If we are going to include text strings in the manifest, they definitely need to be translatable.
    • One possibility I've been considering is to have a locale/xx/manifest.ini file which contains just the translatable strings for each language. The main manifest.ini would still contain the English strings, as they are the default. Having them in separate files makes translation easier when there are multiple people working on a single addon.
  • I'm not sure unloading add-ons or removing add-ons without restart is something we should support at all initially. Aside from the fact that it makes things more complicated, NVDA can't currently unload synth drivers or braille display drivers and we'd have to implement checks to ensure an add-on didn't contain an active driver, etc.
    • I'm not saying we should never support this, but if we do in the initial version, it's going to delay inclusion and imo it isn't that important.

Some code review:

  • In the copyright header, you can specify your own name rather than NVDA contributors. This way, we know who owns the copyright for various sections. Others can be added if others contribute to this file.
  • Any reason you're using StringIO instead of cStringIO? If not, use cStringIO for better performance.
  • I'd prefer the extension ".nvda-addon" instead of ".nvdaadon".
  • I don't think we should allow addons in the source directory. Otherwise, they aren't really add-ons and should be made part of the distribution.
    • That said, you should still leave the support for multiple add-on directories, as we may have a system wide configuration directory eventually.
  • The hook for unloading is named deunload. Should this just be unload?
  • Addon.loadModule creates a full name of addonName.moduleName. I think addons.addonName.moduleName might be better to avoid possible conflicts.
  • We definitely need to find a better way to handle the hack in config.addConfigDirsToPythonPackagePath. Among other things, this can't handle dynamically loading new add-ons at runtime.

Mionr code nits:

  • No need to explicitly import os, as os is implicitly imported by os.path.
  • When a docstring refers to a Python identifier within NVDA, it should link it; e.g. L{Addon}.
  • There's no need for a pass statement if there is a docstring; e.g. the !AddonError class.
  • When documenting that a function raises an exception, the docstring should use @raise; e.g. @raise IOError: If removing a file fails.
  • When raising exceptions, the parenthesised version is preferred over the comma separated version; e.g. raise AddonError("blah") instead of raise AddonError, "blah"

I'll probably have more later.

@nvaccessAuto

Comment 34 by jteh on 2012-04-18 10:43
I forgot to mention that I was also thinking it'd be nice to have gettext translations for add-ons. To do this, there would be a locale directory inside the add-on package with the usual structure (locale\xx_xx\LC_MESSAGES\nvda.mo). There would also be a function (e.g. addonHandler.initTranslation) which would set everything up for the current module so that translatable strings (e.g. _("foo")) would just work. I have ideas on how to do this last part, so am happy to write the code if this idea isn't insane.

@nvaccessAuto

Comment 35 by ragb (in reply to comment 33) on 2012-04-18 11:42
Hi,
Replying to jteh:

This code looks very good and functional. Very nice work.

Thank you.

Questions:

  • What do you imagine hooks being used for? One possibility I see is an installation hook; e.g. to gather data from the user, etc.

That is one of my main use cases. Another one is for nvda to tell the plugin that it's user interface is started so the addon can add to menus, etc. One can also do that using a globalPlugin, but in my opinion that would be clearer. Moreover, beeing hook names strings, add-on developers can define their own hooks, and addons can communicate with one another (do we realy want that to happen? :)). I think the system is extensible for use cases we can't even imagine but if you think that it is too overkill that is fine for me.

Problems:

  • If we are going to include text strings in the manifest, they definitely need to be translatable.

    • One possibility I've been considering is to have a locale/xx/manifest.ini file which contains just the translatable strings for each language. The main manifest.ini would still contain the English strings, as they are the default. Having them in separate files makes translation easier when there are multiple people working on a single addon.

This seems a good idea. However I only see the need to translate description and possibly the name of the addon. But we can think of more and that approach will account for the need.

  • I'm not sure unloading add-ons or removing add-ons without restart is something we should support at all initially. Aside from the fact that it makes things more complicated, NVDA can't currently unload synth drivers or braille display drivers and we'd have to implement checks to ensure an add-on didn't contain an active driver, etc.

Makes sence. I thought about that a bit but was not sure how to approach that. Shall we force a restart on addon installation? If having a GUI to isntall and remove addons, we may only do the restart when changes are "commited" (pressing the ok or save button for instance).

  • I'm not saying we should never support this, but if we do in the initial version, it's going to delay inclusion and imo it isn't that important.

Agreed.

Some code review:

  • In the copyright header, you can specify your own name rather than NVDA contributors. This way, we know who owns the copyright for various sections. Others can be added if others contribute to this file.

Ok.

  • Any reason you're using StringIO instead of cStringIO? If not, use cStringIO for better performance.

No reason that I recall, probably a typo.

  • I'd prefer the extension ".nvda-addon" instead of ".nvdaadon".

Ok, that is good.

  • I don't think we should allow addons in the source directory. Otherwise, they aren't really add-ons and should be made part of the distribution.

My idea when doing that was to allow easy building of portable version, already including some addons. But with the current unified launcher that might impose problems (load the addons already on the temporary copy?). Honestly I now do agree that it doesn't make much sence and if someone needs that later it can be handled at that time.

  • That said, you should still leave the support for multiple add-on directories, as we may have a system wide configuration directory eventually.

I agree.

  • The hook for unloading is named deunload. Should this just be unload?

Yes. Typo. I was calling it deactivate and forgot the "d" there.

  • Addon.loadModule creates a full name of addonName.moduleName. I think addons.addonName.moduleName might be better to avoid possible conflicts.

Good point.

  • We definitely need to find a better way to handle the hack in config.addConfigDirsToPythonPackagePath. Among other things, this can't handle dynamically loading new add-ons at runtime.

Yes. However, if we do the restart at addon loading this might not be a problem, at least for now. But I agree that it is a hack.

Mionr code nits:

  • No need to explicitly import os, as os is implicitly imported by os.path.

  • When a docstring refers to a Python identifier within NVDA, it should link it; e.g. L{Addon}.

  • There's no need for a pass statement if there is a docstring; e.g. the !AddonError class.

  • When documenting that a function raises an exception, the docstring should use @raise; e.g. @raise IOError: If removing a file fails.

  • When raising exceptions, the parenthesised version is preferred over the comma separated version; e.g. raise AddonError("blah") instead of raise AddonError, "blah"

Ok. Thanks.

Regards,

Rui Batista

@nvaccessAuto

Comment 36 by ragb (in reply to comment 34) on 2012-04-18 11:56
Hi again,
Replying to jteh:

I forgot to mention that I was also thinking it'd be nice to have gettext translations for add-ons. To do this, there would be a locale directory inside the add-on package with the usual structure (locale\xx_xx\LC_MESSAGES\nvda.mo). There would also be a function (e.g. addonHandler.initTranslation) which would set everything up for the current module so that translatable strings (e.g. _("foo")) would just work. I have ideas on how to do this last part, so am happy to write the code if this idea isn't insane.

I fully agree with this feature. However we may encounter some problems:

  • The approach people are using to translate plugins is to directly setup gettext on the modules that need translation (pointing it to e specific locale dir and domain). But that has the problem that every module must have some sort of setup, even if it is just one line. That is an opition though.
  • As far as I know, NVDA installs gettext on the __builtin__ module. I don't know if it is possible to have different gettext instances installed for different modules...
  • We could recursively monkey-patch al python modules on the addon to add the _ function, but it might not be diserable to import all python modules at once. In alternative, we can selectively monkey-patch some modules, but don't know what criteria to choose.

Do you have any other idea?

@nvaccessAuto

Comment 37 by jteh (in reply to comment 36) on 2012-04-18 13:21
Replying to ragb:

  • The approach people are using to translate plugins is to directly setup gettext on the modules that need translation (pointing it to e specific locale dir and domain). But that has the problem that every module must have some sort of setup, even if it is just one line.

I agree this is a minor nuisance, but as long as that line isn't too ugly (I'm imagining something as simple as addonHandler.initModuleTranslation()), I think this is an acceptable situation.

  • As far as I know, NVDA installs gettext on the __builtin__ module. I don't know if it is possible to have different gettext instances installed for different modules...

If _ (and potentially other identifiers) are defined in the module, they will override the builtin. The init function will handle this.

  • We could recursively monkey-patch al python modules on the addon to add the _ function, but it might not be diserable to import all python modules at once.

This isn't an option. There could be translatable strings at module and class level. Translation of these will happen at import time, meaning that monkey patching will happen too late.

Do you have any other idea?

One nasty idea I just came up with is to write our own _ function (and put it in builtins) which looks at the stack to determine what module the call came from and selects the correct gettext instance accordingly. I'm not sure about performance here. However, I think I still prefer the initialisation approach.

@nvaccessAuto

Comment 38 by jteh (in reply to comment 35) on 2012-04-18 13:29
Replying to ragb:

  • What do you imagine hooks being used for? One possibility I see is an installation hook; e.g. to gather data from the user, etc.

That is one of my main use cases. Another one is for nvda to tell the plugin that it's user interface is started so the addon can add to menus, etc.

I am against this and think this should still be done in a global plugin. Otherwise, the lines between global plugins, app modules and drivers become too blurred, which could lead to confusion and other problems. Nevertheless, I'm happy to leave the hook mechanism present, even if it's not used much.

  • If we are going to include text strings in the manifest, they definitely need to be translatable.

    • One possibility I've been considering is to have a locale/xx/manifest.ini file which contains just the translatable strings for each language.

This seems a good idea. However I only see the need to translate description and possibly the name of the addon.

Agreed. I know having separate files seems like overkill, but I figured that individual translators would find it easier if their strings were kept separate. I'd appreciate feedback from others on this as well. Doing it that way certainly makes the code more complicated, so I'm happy to keep them all in the one manifest if everyone is happy with that. :)

  • I'm not sure unloading add-ons or removing add-ons without restart is something we should support at all initially.

Makes sence. I thought about that a bit but was not sure how to approach that. Shall we force a restart on addon installation? If having a GUI to isntall and remove addons, we may only do the restart when changes are "commited" (pressing the ok or save button for instance).

Something like that sounds good. Let's cover that when we start working on the GUI. :)

Thanks for your work.

@nvaccessAuto

Comment 39 by ragb (in reply to comment 38) on 2012-04-18 14:10
Replying to jteh:

Replying to ragb:

  • What do you imagine hooks being used for? One possibility I see is an installation hook; e.g. to gather data from the user, etc.

That is one of my main use cases. Another one is for nvda to tell the plugin that it's user interface is started so the addon can add to menus, etc.

I am against this and think this should still be done in a global plugin. Otherwise, the lines between global plugins, app modules and drivers become too blurred, which could lead to confusion and other problems. Nevertheless, I'm happy to leave the hook mechanism present, even if it's not used much.

The only small problem I see with having GUI construction (and destruction if dynamic loading is to be implemented) on a globalPlugin is that we are relying on the fact that the GlobalPlugin constructor is called after GUI initialization. That is, we are relying on module initialization order. At least this should be documented or, in alternative, create something like a {{create_guimethod on theGlobalPlugin``` class for plugins to override, wich is guarantied to be called at the right time, and may be more explicit. This may belong to a new ticket, though.

@nvaccessAuto

Comment 40 by jteh (in reply to comment 39) on 2012-04-18 14:20
Replying to ragb:

The only small problem I see with having GUI construction (and destruction if dynamic loading is to be implemented) on a globalPlugin is that we are relying on the fact that the GlobalPlugin constructor is called after GUI initialization. That is, we are relying on module initialization order.

That's a good point. I can't see any reason we would ever initialise gui after plugins, though, so:

At least this should be documented

I agree.

@nvaccessAuto

Comment 41 by jteh on 2012-04-19 08:43
Rui, are you likely to have a chance to look at implementing these changes over the next few days? If you're busy, I'm happy to do this myself, as I'd like to move this further forward. Thanks!

@nvaccessAuto

Comment 42 by ragb (in reply to comment 41) on 2012-04-19 11:29
Hi,
Replying to jteh:

Rui, are you likely to have a chance to look at implementing these changes over the next few days? If you're busy, I'm happy to do this myself, as I'd like to move this further forward. Thanks!

The code cleanups you sugested and some other corrections are done, I will commit them shortly. I think I can implement the gettext support you sugested this wikend. However, implementing something more complex like a GUI would take a bit more time, so I can not tell exactly when I have time to work on that. If you want to start on that path we could work in paralle, but I don't know if there something more to do before starting working on GUIs and stuff.

regarding manifest translation, I was thinking about something. Supose we want to have some sort of add-on repository on the future. That repository could use manifest information directly to read add-on information. However, if we have translation scathered among many files that sort of processing would be harder. Anyway, this doesn't make things impossible by any means, just a bit more complex. Just a thought.

@nvaccessAuto

Comment 43 by ragb (in reply to comment 42) on 2012-04-19 13:43
Replying to ragb:

The code cleanups you sugested and some other corrections are done, I will commit them shortly.

Pushed to lp:~ruiandrebatista/nvda/addons-packaging. revision 5025.

Regards,

Rui Batista

@nvaccessAuto

Comment 44 by ragb (in reply to comment 42) on 2012-04-20 16:16
Hi,

I coded a prototype for the translation support on lp:~ruiandrebatista/nvda.

  • addonHandler.iniTranslation() does the work of putting _ on the caller globals namespace..
  • addonHandler.Addon.getTranslationsInstance() retrieves the translation for the addon.
  • addonHandler.getCodeAddon() identifies the addon for the caller code (all using stack inspection).
  • Many other changes were needed to support this

This code may be a bit hackish please review... Inspection and such is something new for me so I might be doing things somehow wrong.

@nvaccessAuto

Comment 45 by ragb on 2012-04-24 20:02
Just to note that manifest translation was implemented e r5134 of the launchpad branch. Data in locale\xx\manifest.ini on the addon. See the revision commit message for details.

@nvaccessAuto

Comment 46 by mdcurran on 2012-04-30 05:39
Recently I changed things so that addons that tried to get removed but failed were renamed to blah.removed and then cleaned up on NVDA restart. I think though that either deleting or renaming addon directories is extremely dangerous unless the addon is definitely not being used.
In the ase of a synth driver this is a little tricky, and globalLugins... they can have their terminate method called but there's no guarantee that pulling the files out from underneath it its not going to completely break the running NVDA.
Therefore, I would like to propose a different change:

  • When an addon should be removed, it is simply recorded in a removals list which is stored in a pickle file in the addons dir. On NVDA initialization, the removals list is checked and those addons are removed.
    • If an addon is added, its copied in to the addons dir, but with the string "-pending" appended to its directory name. On NVDA initialization, these directories will be renamed to their real names thus activating them.
  • The Addon class could have two new properties: isNew and isRemoved. isNew would be true if the path has -pending on the end. isRemoved would be true if this addon's name is in the removals list. These properties would then make it possible to manage the GUI better.
  • The Addon class should have a requestRemove method, which if the addon is new, then the -pending directory is simply removed, or if new is false, then its added to the removals list. This also makes it easier to manage from the GUI as newly added Addons can be safely removed.

Thoughts?

@nvaccessAuto

Comment 47 by jteh (in reply to comment 46) on 2012-04-30 09:44
I'm happy with this idea. Just a couple of minor suggestions:
Replying to mdcurran:

  • If an addon is added, its copied in to the addons dir, but with the string "-pending" appended to its directory name.

Consider changing this to ".pending". The reason is that people are less likely to use "." than "-" in add-on names, so we could reserve "." for special things like this.

  • The Addon class could have two new properties: isNew and isRemoved. isNew would be true if the path has -pending on the end.

I wonder whether isPending is a more accurate name here.

@nvaccessAuto

Comment 48 by mdcurran on 2012-04-30 10:16
either isPending, or or us .new instead of .pending for the dir name. I'd possible prefer new over pending as its a little more descriptive ... pending what exactly? :)
Re changing - to a . I agree with that.

@nvaccessAuto

Comment 49 by jteh on 2012-04-30 10:22
The problem with "new" is that it's not clear that it isn't yet actually installed; e.g. it could just be new as in recently added. However, I agree pending is equally ambiguous. Being pedantic, we probably want ".pendingInstall", but I'm happy to go with "new" for brevity as long as the isNew property is clearly documented. :)

@nvaccessAuto

Comment 50 by mdcurran on 2012-04-30 10:24
I don't mind .pendingInstall and isPendingInstall at all. I think it says what it does, and really length isn't that much of an issue.

@nvaccessAuto

Comment 51 by jteh on 2012-04-30 10:27
Sweet. Ship it!

@nvaccessAuto

Comment 52 by mdcurran on 2012-04-30 11:07
Your thoughts on the change Rui?

@nvaccessAuto

Comment 53 by ragb (in reply to comment 52) on 2012-04-30 11:56
Replying to mdcurran:

Your thoughts on the change Rui?

This seems very good to me. The removals list avoids those problems you mention about path changing with code running and really simplifies the gui implementation.

For me .pendingInstall extension and isPendingInstall property seem the better option.

Did you saw my comment on the mailling list about using manifest translated data on the gui?

@nvaccessAuto

Comment 54 by jteh (in reply to comment 53) on 2012-05-01 04:48
Replying to ragb:

Did you saw my comment on the mailling list about using manifest translated data on the gui?

You wrote:

I couldn't get with a good way to merge the translation with the original manifest data, so I implemented a separate _ call.

Can you explain what complications (if any) you hit when trying to do this? I imagined something like:

for field in ("description", "long_description"):
 self[= transConf[field](field])

Btw, Mick did reply to your message on the list and suggested manifest.update(languageManifest). However, this would override all keys instead of just the localised ones, hence my suggestion above.

That is also more explicit in the sence that people understand those are localized strings.

True, though I'm not sure whether this justifies the extra code, given the simplicity of the above solution (assuming i haven't missed something). Thoughts?

Btw, there's probably no need to have a configSpec for the translated config, as it won't get used anyway unless you validate it and the values are all strings anyway.

With regard to your !AddonManifest._ method, can you explain why the method takes a list of keys? I don't follow this, as it then only returns one value.

@nvaccessAuto

Comment 55 by ragb (in reply to comment 54) on 2012-05-01 11:20
Replying to jteh:

Replying to ragb:

Did you saw my comment on the mailling list about using manifest translated data on the gui?

You wrote:

I couldn't get with a good way to merge the translation with the original manifest data, so I implemented a separate _ call.

Can you explain what complications (if any) you hit when trying to do this? I imagined something like:

for field in ("description", "long_description"):
 self[= transConf[field](field])

The complications I had were that I tried to write the code to be extensible for subsections. That is, if our manifest data had subsections to be translated. Anyway... ok, those are just dictionaries. If we don't write-back the manifest data there is no problem merging them.

Btw, Mick did reply to your message on the list and suggested manifest.update(languageManifest). However, this would override all keys instead of just the localised ones, hence my suggestion above.

I didn't read is message fo some reason... will recheck.

That is also more explicit in the sence that people understand those are localized strings.

True, though I'm not sure whether this justifies the extra code, given the simplicity of the above solution (assuming i haven't missed something). Thoughts?

Btw, there's probably no need to have a configSpec for the translated config, as it won't get used anyway unless you validate it and the values are all strings anyway.

Yes, I will delete that.

With regard to your !AddonManifest._ method, can you explain why the method takes a list of keys? I don't follow this, as it then only returns one value.

It is to be extensible for subsections. Was thinking on adding an extra section to the manifest so wrote that that way, but I droped the idea because it wasn't making much sence, for now.

So, as you pointed above explicitly copying some fields may be the best solution for now (if in the future we need more complex data to be translated we can merge the translated config and, if needed, validate it to make sure it as no extra-fields). Do you agree?

@nvaccessAuto

Comment 56 by jteh on 2012-05-01 11:25
Mick changed the code today to merge explicit fields. If we need to localise sub-sections in future, we may need to consider a more elaborate solution (maybe even something closer to what you originally had), but I think it's best not to over-complicate it for now. Extensibility is nice, but only if you can anticipate the kind of extension that will occur. :)

@nvaccessAuto

Comment 57 by ragb (in reply to comment 56) on 2012-05-01 11:28
Replying to jteh:

Mick changed the code today to merge explicit fields. If we need to localise sub-sections in future, we may need to consider a more elaborate solution (maybe even something closer to what you originally had), but I think it's best not to over-complicate it for now. Extensibility is nice, but only if you can anticipate the kind of extension that will occur. :)

Yes :). I'm now reading mike's commits.

@nvaccessAuto

Comment 58 by jteh on 2012-05-02 04:37
One point that needs to be decided is how to handle addon specific installation tasks (e.g. prompting for registration details for commercial synths, etc.). There is the hooks mechanism, but it's not quite so simple now, as the add-on isn't really usable in the pendingInstall state. If we have an install hook, the problem is that the hooks module might try to import modules that aren't yet accessible.

Mick and i were discussing the possibility of having a separate installHooks (or installTasks) module. Separating it like this means you can document clearly that other modules in the add-on aren't yet usable. This way, there could be an install hook which is run after the add-on is extracted. If it fails, the extracted files would be immediately removed and the installation would fail.

One problem is how to handle this for uninstallation. If an uninstall hook wants to remove or modify files, it can only do this as the add-on is being removed once NVDA restarts. If the uninstall hook wants to display some GUI, this means that the removal of add-ons has to be delayed until after the main loop has started.

Rui, your thoughts would be greatly appreciated. :)

@nvaccessAuto

Comment 59 by ragb (in reply to comment 58) on 2012-05-02 10:47
This definitly requires some thought, but my first ideas are as follows
Replying to jteh:

One point that needs to be decided is how to handle addon specific installation tasks (e.g. prompting for registration details for commercial synths, etc.). There is the hooks mechanism, but it's not quite so simple now, as the add-on isn't really usable in the pendingInstall state. If we have an install hook, the problem is that the hooks module might try to import modules that aren't yet accessible.

If we have an addon instance this means that the addon is extracted. So, if we have access to the hooks modules other modules can be imported, from there. Am I not seeing anything? :) probably yes :). Obviously no plugin nor braille display is running, but wx and such can be used. if the addon is to modify anything on installation one must have in mind that paths will change and such. But, as things are simply renamed, there should not be a big problem in most cases.

Mick and i were discussing the possibility of having a separate installHooks (or installTasks) module. Separating it like this means you can document clearly that other modules in the add-on aren't yet usable. This way, there could be an install hook which is run after the add-on is extracted. If it fails, the extracted files would be immediately removed and the installation would fail.

I have no problem with that but the hooks module already can contain one hook that runs before many things: the "load" hook. Honestly I only used it for debugging porposes but at the time this is called there isn't much stuff initialized in NVDA, I think. I'm happy to remove this hook if noone finds any utility for it, further then debugging. Maybe we should document the conditions when a hook is called. I thinkk that the "install" hook can be on hooks.py without much trouble, but if you think that installTass.py or something would be more explicit I'm also happy with that.

One problem is how to handle this for uninstallation. If an uninstall hook wants to remove or modify files, it can only do this as the add-on is being removed once NVDA restarts. If the uninstall hook wants to display some GUI, this means that the removal of add-ons has to be delayed until after the main loop has started.

Yes. I don't see any other solution either then delaying it... But it's not that beautiful though. Another issue is wat to do if uninstallation throws any error or something. Remove the addon in any case?

Rui, your thoughts would be greatly appreciated. :)

I didn't have time to thing that much about this, but here they are...

@nvaccessAuto

Comment 60 by jteh on 2012-05-07 23:06
Sorry to be painful, but addon in the UI and documentation should really be spelled as add-on.

@nvaccessAuto

Comment 61 by jteh on 2012-05-08 00:36
Just a minor UI point. Should "Add-ons Manager" in the Tools menu instead be "Add-ons" or "Manage add-ons"; i.e. a description rather than a title?

@nvaccessAuto

Comment 62 by jteh on 2012-05-08 08:57
Merged in de41cb2. Defects or enhancements should be filed in new tickets.

Many thanks to Rui for all of his work on this.
Changes:
State: closed

@nvaccessAuto nvaccessAuto added this to the 2012.2 milestone Nov 10, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment