less: generate map files when building style.css with 'npm run less'
The map files allow web development tools ("Firebug" style) to show the .less and .css source instead of the generated style.css . The generated style.css will get a trailing comment with a reference to kallithea/public/css/style.css.map . It will not have any impact on runtime.
1) Not using 'async defer' - it's not used anywhere else, plus I have no idea of what exactly it does.
2) The recaptcha div has an ID so the label for="recaptcha_field" can reference it (although I'm not certain a div can technically have a label, but it's better than removing the label, or the for="").
graph: make sure graph is shown next to the table it belongs to
For some screens sizes (for example around 800 px), the graph with changes available for PR update wasn't shown next to the corresponding table. The two boxes would float vertically and be rendered after each other.
To fix that, use the same trick as before 02aef1484695: set the graph height to 0 and give the table a left margin leaving room for the graph.
(The fancyness of Bootstrap grids does in this case seem to only get in the way. Plain percent would perhaps work better.)
A plain "pip install -e ." would obey the Kallithea setup.py and install WebOb 1.8.0, but fail at runtime with: (WebOb 1.8.0, Requirement.parse('WebOb<1.8.0,>=1.2'), set(['TurboGears2']))
issues: support generic regex replacements in issue_url and issue_prefix
Issue reference linking is pretty limited: - the issue_url is a literal with only three special tokens {id}, {repo} and {repo_name}. There is no way to let the URL be dependent on other elements of the input issue reference. - The value for {id} is somewhat oddly determined by the concatenation of all parenthesized groups in the issue_pat regular expression - the link text of the resulting link is limited to the contents of the literal issue_prefix with the determined {id}. It is not possible to retain the input issue reference verbatim, nor to let the link text be dependent on other elements of the input issue reference.
This commit makes the issue reference linking more flexible:
- issue_prefix is replaced by the more generic issue_sub(stitution), which is a string that may contain backreferences to regex groups specified in issue_pat. This string, with backreferences resolved, is used as the link text of urlified issue references. - if issue_sub is empty, the entire text matched by issue_pat is used as the link text. - like issue_sub, also issue_url can contain backreferences to regex groups. - {id} is no longer treated as a special token, as it can be solved by generic backreferences ('\g<id>' assuming issue pattern contains something like '(P<id>\d+)'. {repo} and {repo_name} are still supported, because their value is provided externally and not normally part of the issue pattern.
Documentation and ini file template is updated as well.
This is essentially a backout of commit 32e1e0745d3c.
That commit checked for whitespace at the beginning of the matched issue reference, and explicitly retained it in the resulting link text.
The way this was handled is not only suboptimal, e.g. a set of 4 spaces would still be reduced to 1, but is also not actually necessary: if whitespace before the issue reference is not required, then it does not need to be specified in the issue pattern, and if it _is_ required, then a positive lookbehind assertion can be used instead.
Add two test cases using the default issue pattern from the ini file template prior to commit 9cef5615da7b, and some more cases, some of which verify the behavior regarding test cases with mandatory/optional leading whitespace.
Increase readability by grouping tests with the same issue pattern, server and prefix, and place the urlified result on a separate line from the input, wrapped in case there are multiple issues in the input.
For the result, """ string delimiters are used instead of ' as used in the input, to visually distinguish them better.
less: style panel headings with variables the same way as the (inverse) navbars are styled
No visible change by default, but splitting these variables out separately and explicitly makes the system more flexible and prepare for future changes or customizations.
@panel-primary-heading-bg already indirectly defaulted to @kallithea-theme-main-color.
@panel-primary-text defaulted to white and would have to be customized anyway if using a light background.
tests: vcs: recreate_repo_per_test is really optional
While the documentation says this attribute is required, most tests do not set it when the default value is fine. Follow-through that convention and update the documentation.
tests: vcs: use pytest fixtures instead of xUnit-style setup_method/setup_class
A subsequent commit will introduce parametrization of fixtures to avoid each test class to be duplicated for every supported VCS explicitly.
But this concept does not mix with xUnit-style methods setup_class and setup_method, because the latter are called much earlier than pytest fixtures and thus cannot use variables set only later by a fixture.
Therefore, use a real fixture to set up the class and test methods.
test_changesets.py needs to create a repository, and is currently duplicating some of the base setup code.
Instead, split out the setup_class method to allow reuse.
Note that the repo_path variable previously registered to 'self' is not actually used by anyone, and moreover is not actually necessary as the path can be obtained via the repo object too. Therefore, just make it a local variable.
tests: vcs: use _BackendTestMixin instead of duplicated BackendBaseTestCase in test_filenodes_unicode_path
test_inmemchangesets.py hosts a second base class in addition to _BackendTestMixin, which is basically a duplication we want to get rid of.
As that duplicated base class BackendBaseTestCase is still in use by test_filenodes_unicode_path.py, we first need to let that test use the main base class.
make-release: import version and copyright updates from default branch (dba4e770d4b6)
make-release has been improved on the default branch in commit dba4e770d4b6, and that version of the script also almost works on stable.
There are some parts we don't have in stable branch: dev_requirements.txt, run-all-cleanup, and a working dist file content check:
The MANIFEST.in file on the stable branch is not complete. Most of the missing entries could simply be added (.travis.yml, requirements.txt, tox.ini, and scripts/*) but there is one problematic entry: kallithea/i18n/en/LC_MESSAGES/kallithea.mo. In the check command, all .mo entries are filtered out, causing a remaining diff for the English .mo.
As that file has been removed on the default branch already, and the missing entries are not important for the release anyway, simply drop the check for now.
tests: Remove metaprogramming constructs for vcs test classes (issue #309):
- Removed use of the globals() and type() constructs to programatically instantiate Git/Mercurial-specific test classes. This should make it a bit clearer what tests are being run at the expense of possible future VCS additions. - Removed the SCM_TESTS VCS test configuration variable, since it got removed. Previously it was used for instantiating test classes. - Updated small snippet of inline documentation that described the use of SCM_TESTS variable. New text points to inheriting from generic test classes instead. - base.py had a dead snippet - kill it.
git: fix URL for submodules - make it link to the external URL
Without this changeset, the link on the files page would not point to the submodule target. Instead it would simple point to ./<submodulename>, which when clicked leads to a '500 Internal Server Error'.
DEBUG and INFO are not good choices for the default log levels. With DEBUG and INFO most of the time you can't see the wood for the trees. It is too easy to overlook critical errors or warnings if the log levels DEBUG and INFO are enabled.
Using a fixed width doesn't make good use of the available space. On small screens the graph uses too much space, so the other columns can not be displayed. On big screens there'd be more space to display more parallel branches in the graph.
We already have a loop for adding Ui entries, so why not handle the hook entries there as well?! This is also much easier to read. create_settings is only called from setup_db so querying for existing settings makes no sense.
hooks: rename hooks to reflect what they are doing
The names of our hooks should reflect what they are doing. So, since pre_push and pre_pull only handle the locking, rename pre_push->push_lock_handling and pre_pull->pull_lock_handling.
This imply a database migration step which must be run as described in the upgrade documentation.
hook: the git hooks should exit with the return value of the handlers
Like with mercurial hooks, returning a non-zero value means abort the operation (if possible). So, if for example a hook returns a non-zero value to abort an unauthorized push, the git hook script needs to exit with that value.
pullrequests: fix broken delete button on overview list
428c1193ee40 changed the "Delete" button on the pull request overview page to no longer be a "submit" button. That broke the functionality: In order for the form to be submitted, the button needs to be of type submit.
comments: don't leave stray "Add Another Comment" buttons after cancelling adding comments on a new line
2e72d2d16a0f introduced an invisible "submitting" div above comment forms. That div was included when counting the number of comments on a line, and it thus broke the logic for removing the last DOM elements after cancelling comments on a new line.
To fix that, exclude the submitting div in the selector used for counting comments.
less: override some bootstrap less variables to make headers more compact (waste less space)
- make navbar smaller by reducing navbar-height - less space between navbar items by setting a smaller nav-link-padding (and thus no need for a custom navbar-padding-vertical) - make panel headers smaller by reducing panel-heading-padding
Commit 494c793cc160 changed HTML escaping to please HTML 4 email readers. The HTML entity ''' was replaced by '''. Unfortunately, the pound character '#' is often used to mark issue references, like 'bug #56'. While this depends on the issue patterns actually configured, this pattern is so common that we cannot expect users to set their issue_pat regular expressions such that '{' is not matched.
Instead, keep the original ''' replacement at first in method html_escape, but introduce a final step that just replaces ''' with '''.
The order of replacement in urlify_text then changes from: html_escape (to HTML4) urlify_issues to html_escape (to HTML5) urlify_issues make HTML5 more like HTML4
tests: issues: use urlify_text wrapper rather than urlify_issues directly
urlify_text is a wrapper around different kinds of urlification methods. The other tests for urlification use that wrapper, while the new urlify_issues tests introduced in commit b43e77fa50dd used urlify_issues directly.
By itself this is not a real problem, but it does not allow to detect the interplay between different urlification issues. For example, an issue pattern normally detecting #123 conflicts with the escaping of an apostrophe ' into ' . Some new tests exposing this problem are added.
diff: fix 2-way diff panes not being shown side by side
eee7674cef41 had the side effect of setting overflow styling on the DOM elements used by mergely. That confused it's computation of sizes on some browsers, scroll bars got added, and the panes were re-flowed to not be side-by-side.
To fix that, add an exception that allow overflow-x to be set to visible.
email templates: reorder references to pullrequests
The current references to PRs in email templates are:
Pull request from <repo> branch <branch> to <repo> branch <branch>: <niceid> <title> by <user>
as the first line can become very long and therefore hard to read, a first step is to introduce a newline before the destination branch:
Pull request from <repo> branch <branch> to <repo> branch <branch>: <niceid> <title> by <user>
But it would be nicer if the from/to part aligns more. Moreover, the most useful part is (hopefully) the PR title, the from/to is secondary and can sometimes even be inferred from the title and context the recipient is already aware of.
Thus, this commit proposes the following format:
Pull request <niceid> <title> by <user> from <repo> branch <branch> to <repo> branch <branch>
This adds some divergence with the changeset references, which could be aligned later if desired.
email templates: align word to indicate branches: at / branch
Some email templates use '<repo> at <branch>' while others use '<repo> branch <branch>'. Interestingly even some txt templates use one and the corresponding HTML template uses the other.
This commit aligns all uses of 'at' and 'branch' to 'branch' as it is considered more expressive. The other choice would also be possible, as long as all users are aligned.
HTML email templates: restrict amount of visible hyperlinks
Users report that some HTML email templates are confusing due to the large number of hyperlinks in them. They typically click the first thing they see that looks clickable without actually reading what they are clicking on, expecting that it all points to the same thing.
In pullrequest invitations, the pullrequest and changeset URLs should be clickable, but the source and target repository should not: this is not what you typically want to see when taking action on this mail.
Similarly in comment emails, the corresponding repository should not be clickable.
Unfortunately, even if we remove the <a> hyperlink tags on these repository URLs, most mail clients will still recognize the text as being URLs and make them clickable anyway. Worse, they will do so in a different styling than the other links.
Therefore, do add <a> tags in the HTML, but make use of styling like that of data_style, i.e. regular text color, grey background with dark grey border. The links _will_ thus be clickable but it will not be as inviting as before, thus removing confusion.
HTML email templates: don't use link color for PR descriptions
The link color should only be used for actual hyperlinks. Using it for anything else is very confusing for users.
For pullrequest mails, the usage of the link color for the PR description may have been done to separate the description more clearly from the title word 'Description' above it. Instead, use a table with grey background similar to how comments are displayed. This meets the objective of not abusing link colors as well as clearly separating the description from the rest of the mail.
The example setting from the default ini file is: issue_pat = (?:\s*#)(\d+) issue_server_link = https://issues.example.com/{repo}/issue/{id} issue_prefix = #
in which the clause '(?:\s*#)' is overly complex: 1. '\s*' means optional whitespace, which means there can be whitespace or no whitespace, which means that you could equally write: (?:#)(\d+) (If any leading whitespace was eaten by the regexp, 32e1e0745d3c would take care to restore a part of it.) 2. '(?:xxx)' means a non-capturing set of parentheses. In this case this is equal to just mentioning 'xxx'. So the simplified pattern becomes: #(\d+)
If instead of _optional_ whitespace, _mandatory_ whitespace was intended, then the pattern should be different. But this would also mean that patterns like PR#123 would not be matched anymore.
Commit 39a59e6915bb398b42c3c2a63c48a950e9d63b55 (helpers: refactor and optimize urlify_issues) made issue_prefix mandatory, while previously it could be empty. An empty issue_prefix is useful when the entire issue pattern needs to be used in the created link.
- move .changeset-logical-index style to .table#changesets block - there is no .changeset_id - there is no #changeset_compare_view_content .compare_view_commits
lib: change ' to ' to satisfy Outlook HTML rendering
The HTML entity ' (') did not exist in the HTML 4 spec [1] and was only added later. As Outlook (and Thunderbird) uses an old engine to render HTML and CSS, it does not recognize this entity and treats it as a literal string.
Therefore, use the equivalent ' code which should be recognized by all browsers, even those restricted to HTML 4.