-
-
Notifications
You must be signed in to change notification settings - Fork 626
-
-
Notifications
You must be signed in to change notification settings - Fork 626
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
Unified support for add-on documentation #2694
Comments
Comment 1 by norrumar on 2012-10-03 19:45 |
Comment 2 by norrumar on 2012-10-04 21:44
|
Comment 3 by jteh on 2012-10-09 06:40 getDocFolder() should probably resort to English if there are no docs for the current locale. I also wonder whether it should raise an exception or return None if the folder doesn't exist. Why is isDocFolder() necessary? Imo, the add-on shouldn't need to ask the question. The author knows whether it has documentation, so the code doesn't need to ask this. docPath() should probably be renamed to getDocPath(). Also, I think it should take a file name, as the author may wish to have multiple documents. I guess this can default to readme.html, though. Minor point: getDocFolder() shouldn't calculate tryLang unless it is necessary. Right now, it is calculated even if it isn't used. |
Comment 4 by norrumar on 2012-10-09 09:32 I'm not sure about this point. For instance most of my add-ons have documentation only in Spanish or other languages. I don't know if English sould be set as default language. "I also wonder whether it should raise an exception or return None if the folder doesn't exist. In my add-on for eMule, I use insert+h for opening a file containing help. In this module I included an exception, but getDocFolder perhaps should return None, because then you can raise different exceptions. It's just an idea. |
Comment 5 by norrumar on 2012-10-09 19:43
|
Comment 6 by jteh (in reply to comment 5) on 2012-10-10 06:51
Because NVDA's primary language is English, this bias already exists in the code. For example, strings enclosed in _() are expected to be English. The user shouldn't notice this, of course, but it's consistent in the code.
Hmm. Tricky. For NVDA's own documentation, it always resorts to English if no documentation is available in the user's language. While this might seem odd, it means that the user at least has the option of reading the documentation in English (assuming they can understand it), rather than being given no access at all because it hasn't been translated. I'd like to hear others' thoughts on this for add-ons. In any case, isDocFolder should probably be renamed to hasDocFolder or maybe even just hasDoc. Replying to norrumar:
This seems unnecessarily complicated. Imo, it only needs to take a file name. The add-on author should know the name of the file and translators should be producing documentation in the same format. |
Comment 7 by ragb (in reply to comment 6) on 2012-10-10 14:00
IMO, it makes no sence to not have documentation in english, if it is to be translated somehow. And english seems to me the only reasonable defaut to revert to, if the user's language is not available. What I'm trying to say is that I strongly recomend if the add-on has documentation it has to be writen at least in english. If the author is not a native speaker, I supose there are people available to take a look at gramar and such, if needed.
I fully agree with this. It's better english then nothing at all, by all means.
Agree.
I agree. The only suitable default I can think of is My only concern with this aproach of beeing the add-on author responsible of adding items to the help menu, is that the order of items in this menu will not be previsible. The problem is that there may not be a suitable order actualy. I'd say alfabetical order of add-on name, but that is just random too. |
Comment 8 by norrumar on 2012-10-10 18:09 |
Attachment 2694.diff added by norrumar on 2012-10-10 18:11 |
Comment 9 by MHameed on 2013-05-20 06:02 |
Comment 12 by jteh on 2013-05-29 02:05 @norrumar, can you please provide your full name so we can acknowledge you as a contributor and copyright holder for this code? Thanks. |
Comment 14 by norrumar on 2013-06-17 19:53 |
Comment 15 by jteh on 2013-06-20 04:43 |
Comment 16 by MHameed on 2013-06-20 06:56 Nolia and I have sorted out the automatic generation of html documentation with no additional work to translators, so the task is almost complete from this side. |
Comment 17 by jteh on 2014-02-21 03:09 The global plugin in the add-on template really needs to be removed, as it affects performance, both at startup and every time NVDA processes a key press or event. There's no need for the documentation for an app module to be available even when the application isn't running. It could be made into a utility module which gets used by add-on code. |
Comment 18 by norrumar (in reply to comment 17) on 2014-02-21 04:28
|
Comment 20 by ateu on 2014-02-22 12:28 |
Comment 21 by norrumar on 2014-02-22 16:17
|
Comment 22 by norrumar on 2014-02-23 05:53
|
Comment 24 by jteh on 2014-05-26 04:15 First, thanks Noelia for your work on this. Sorry it's taken us so long to actually do anything useful with it. I'm going to try to devote some time to code review each week from now on, so hopefully, we can get this moving a bit. Regarding the current code:
Are there any major objections to accessing the documentation from the Add-ons Manager as outlined in #3917? I suggest, though, that we have a separate "Help" or "Documentation" button in the Add-ons Manager to avoid having to press "About" first, thus eliminating one step. |
Comment 25 by norrumar (in reply to comment 24) on 2014-05-26 05:14
OK, it seams reasonable.
OK, I agree. docFileName is better.
I strongly agree with you about accessing documentation from add-ons manager with a separated button, without pressing About. |
Comment 26 by jteh on 2014-05-27 07:58 There is now just one method on addonHandler.Addon called getDocFilePath. It takes an optional file name, defaulting to the docFileName parameter in the manifest. docFileName defaults to None. I'm not sure if anyone will ever want to open a file other than the default. If they do, it's possible that only some of the documentation is localised for a given language, so it needs to fall back to other languages on a per-file basis, not a per-directory basis. I also figured this is more useful than getting the documentation folder alone. I removed the method to open the documentation because the caller might have already retrieved the file path to check whether it exists, so getting it twice is wasteful. Also, they might want to handle errors in a specific way. If you think this is still necessary, let me know. The Add-on help button in the Add-ons Manager will open the default documentation file if there is one. If there isn't, the button will be disabled. Does this work for everyone? |
Comment 27 by jteh on 2014-05-27 07:58 |
Comment 28 by norrumar (in reply to comment 26) on 2014-05-30 15:10
For me it's right now. |
Comment 29 by jteh (in reply to comment 28) on 2014-06-02 03:39
I don't have the original version of your patch, so I can't quite remember why I said this. I wonder whether your original code was testing for the existence of the directory even though an earlier directory existed. Adding a string to a list isn't an issue, but unnecessary file system checks are a bit more expensive. Perhaps you didn't do this and I was just being overly paranoid. Either way, the code you needed in the end to work around it (multiple checks and break statements) was more confusing than just adding the string to the list, so I think adding a string to a list is better. I apologise if I indeed gave you the wrong advice earlier. :)
What you're doing there is converting an ANSI path to a Unicode path, since Python modules use ANSI paths. However, the paths returned by Addon.path are already Unicode. Out of interest, how does docHandler fail without the decode? Most functions should be able to accept both ANSI and Unicode paths. |
Comment 30 by norrumar (in reply to comment 29) on 2014-06-02 05:47
OK, I don't have my original code, but I agree with you. In this case, your code is right for me.
I can't test why docHandler doesn't work withouu ansi to unicode conversion, since my name has just latin characters. However, I test your t2694 branch using non latin characters in docFileName and it works perfectly. |
Comment 32 by James Teh <jamie@... on 2014-06-03 04:31
Changes:
|
Comment 33 by James Teh <jamie@... on 2014-06-24 08:56
Changes:
|
Comment 34 by jteh on 2014-06-24 08:58 |
Comment 35 by driemer.riemer@... on 2014-08-20 02:57 |
Comment 36 by jteh on 2014-08-20 06:00 |
Comment 37 by driemer.riemer@... on 2014-08-20 06:32 |
Reported by jteh on 2012-10-01 02:41
It's already possible for add-ons to provide their own documentation. However, there's no unified support or documentation for this, which makes it inconsistent and difficult and means constant reinvention of the wheel.
I think plugins should still add their documentation to the Help menu themselves. However, it might be good to have functions in addonHandler to make retrieving the path to localised documentation files easier and to document (probably in the Developer Guide) where add-on documentation should be placed.
Blocking #3917
The text was updated successfully, but these errors were encountered: