hooks: move the vcs hook entry points and setup code out of lib
Mercurial hooks are running in a process that already has been initialized, so they invoke the hooks lib directly. Git hooks are binaries and need a lot of initialization before they can do the same. Move this extra setup code elsewhere.
Having this high level code in bin is perhaps also not ideal, but it also doesn't seem that bad: that is where other command line entry points invoke make_app.
(It seems like it could be adventageous to somehow use "real" bin commands for hooks ... but for now we use the home-made templates.)
Note: As a side effect of this change, all git hooks *must* be re-installed when upgrading.
git: detect existing symlink hooks before overwriting - only update plain files
If the existing hook is a symlink, the hook is not what we put in place, and we should stay away - no matter if it points at something that looks like a Kallithea hook.
*If* there should be circular dependencies, importing 'from' another module could fail because the module at that time only was partially imported. That had to be worked around by importing at runtime instead of globally.
Instead, try to always import whole modules. (But we should still try to avoid cycles.)
It might be a good idea, but then we should use it much more consistently ... and it should probably be done differently. Let's keep it simple and be consistent.
lib: move locale.py to locales.py to avoid shadowing of standard module
"Fix" spurious problem, seen for example as:
$ python kallithea/lib/annotate.py Traceback (most recent call last): File ".../lib64/python3.8/site-packages/mercurial/encoding.py", line 107, in <module> encoding = locale.getpreferredencoding().encode('ascii') or b'ascii' AttributeError: module 'locale' has no attribute 'getpreferredencoding'
That happened when something in some other module tried to import stdlib locale ... but somehow would pick up the kallithea locale module and things would fail.
Stay out of that kind of trouble by using a name that doesn't collide.
diffs: remove unused argument enable_comments and class no-comment
enable_comments was only used to set/not-set the 'no-comment' CSS class. While this class was emitted, no CSS rule nor any JavaScript logic was actually using it. Last real usage of that class was removed with commit e87baa8f1c5bd2488aefc23b95c0db3a04bc8431.
Cleanup the code by not emitting 'no-comment' and remove the 'enable_comments' flag.
style: mark failed comment submissions with red panel heading
Make it more obvious to the user that a comment submission failed: mark the panel of the failed comment as "panel-danger" so the color of the comment panel heading changes to red.
Previously, only the user and comment text would fade a bit.
lib/diffs: make sure that trailing tabs are indicated
Between the initial submission and final version of commit f79c40759d6f, changes were made that turn out to be incorrect. The changes assume that the later match on trailing tabs will 'win' from the plain 'tab' match. However, Python 're' documentation says:
As the target string is scanned, REs separated by '|' are tried from left to right. When one pattern completely matches, that branch is accepted. This means that once A matches, B will not be tested further, even if it would produce a longer overall match. In other words, the '|' operator is never greedy. https://docs.python.org/3.8/library/re.html
As a result, a trailing tab is seen as a plain tab and not highlighted in a special way.
Unify the tab handling to make it unambiguous how they should be parsed.
The change diff mainly shows re group numbers shifting.
It is checked earlier that git_command is one of two string constants, and with py3, things are much simpler and we don't have to consider string coersion.
use.py: import re import sys for fn in sys.argv[1:]: with open(fn) as f: s = f.read() s = re.sub(r'''(<script>)('use strict';)\n( *)''', r'''\1\n\3\2\n\3''', s) with open(fn, 'w') as f: f.write(s)
python use.py $(hg loc 'kallithea/templates/**.html')
config: move WSGI middleware apps from lib to config
These middlewares are full WSGI applications - that is not so lib-ish. The middleware is referenced from the application in config - that seems like a good place for them to live.
config: move various py templates to kallithea/templates/py/
For some reason, we had some python templates in kallithea/config . kallithea.config is mainly the TG entry point, and thus a high level controller thing - not a place to store templates.
Instead, use the templates directory and introduce a new py subdirectory.
With git hook templates in a templates directory, there is no need for tmpl in the name.
routing: move config.routing to kallithea.controllers
Routing doesn't belong in config. Having it there caused unfortunate dependencies.
We do routing the old way. If we did it the new way, it would be defined in the root controller. But for now, we just place it next to the root controller.
scm: make sure __get_repo always returns something
Avoid pytype complaint: File ".../kallithea/model/scm.py", line 438, in get_nodes: No attribute 'scm_instance' on None [attribute-error] In Optional[Any]
Avoid invoking __get_repo with None as parameter when creating repos.
ssh: import binascii directly, instead of using it through base64 module
It is unfortunate that the base64 module is leaking its binascii internals in exception types. We started using binascii through the base64 import in 08af13a090e0, but the import is not public, and pytype thus complains.
When browser zoom is used, the font size increases. As a result, the text balloon icon used in the comment bubble increases as well. However, the rectangle in which the icon resides is kept the same size, causing the result to look bad.
Use 'em' units, which are relative to the font size, rather than pixels, for all relevant style properties of the bubble, including size, padding and border-radius. With these settings, the bubble always looks well, regardless of zoom value.
api: stop using 'Optional', 'OAttr'/'OptionalAttr' classes
There does not seem to be a good reason to use the 'Optional' and 'OptionalAttr' classes. It makes the code harder to understand. And worse, the 'default value' specified is not always used, which can thus give false information to users.
The way Optional was used in the API calls is twofold:
1.either by effectively extracting a value, via Optional.extract(param). If 'param' was indeed specified by the user, then this would yield that user-specified value. Otherwise, it would yield the value declared in the parameter declaration, e.g. param=Optional(defaultvalue).
2.or by checking if a variable is an instance of the Optional class. In case a user effectively passed a value, this value will not be of the Optional class. So if a parameter is an object of class Optional, we know the user did not pass a value, and we can apply some default.
In the declaration of the parameter, the specified default value will only be used if the 'extract' method is used, i.e. method 1 above.
A simpler way to address this problem of default values is just with Python default values, using 'None' as magic value if the default will be calculated inside the method.
The docstrings still specify something like: type: Optional(bool) which is humanly readable and does not necessarily refer to a class called 'Optional', so such strings are kept.
Allow adding and removing reviewers of a pull request with the API call 'edit_reviewers', taking a pull request ID and a list of usernames or userids to add and remove. For single-user operation, a string can be used instead of a list.
Note that the 'kallithea-api' tool only accepts strings, so can only perform single-user operations. Use python 'requests', 'curl' or similar instead if you need to operate on multiple users at a time.
extensions: rename 'rcextensions' into 'extensions' but provide compatibility
The 'rc' prefix is legacy. Rename 'rcextensions' into 'extensions', updating all references. Compatibility with the old name will be retained for a while, and removed in a later release. Migrating is as simple as renaming a file.
- use 'try..except' rather than 'if-then', which removes the need for a separate 'path' variable and is more 'pythonic'. - promote log from 'debug' to 'info' and execute it immediately after loading before anything else. - clarify comments related to INDEX_EXTENSIONS
There seems to be no reason that in indirection is needed between the actual hook name and the implementation. Moreover, the implementation names were unnecessary complex.
db: introduce constraint ensuring no duplicate (reviewer, pullrequest) combinations
A reviewer should only be added once to a review.
Previously, this was not ensured by the database itself, although that the controller would try to not add duplicate reviewers. But there was no hard guarantee: e.g. simultaneous adding of the same reviewer to the same review by a review owner and admin, a framework bug that sends the same request twice, ... could still trigger duplicate addition. Additionally, code changes (e.g. a new API) could introduce bugs at the controller level.
Existing production databases were found to contain such duplicate entries. Nevertheless, as the code displaying reviewers in a pull request filtered out duplicates, this never showed in the UI, and never was a 'real' problem.
Add a UniqueConstraint in the database to prevent such entries, with a database migration step that will first find and remove existing duplicates.