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.
vcs: fix get_changesets filtering on hg repo to AND the criteria instead of OR
Mercurial scmutil.revrange takes a list of filters ... and OR them. But when for example a user uses the api and sets branch name and date, he would expect to only get revisions from the provided branch. So we need to use AND when filtering.
When using AND, the special handling of start_date and end_date is no longer necessary.
vcs: introduce 'branches' attribute on changesets, making it possible for Git to show multiple branches for a changeset
Mercurial changesets will always have have exactly one branch (which might be "default"). The VCS data model was the same.
Git allows for a changeset to have 0 or more branches ... and possibly one of them as active. The right data model is thus to have an enumerable of branches.
We thus add a 'branches' attribute and use it where applicable.
The existing 'branch' attribute used some heuristics to decide which branch use as "the" branch ... and in some places code (and tests) rely on that. We thus keep that old method, knowing that some of its uses probably should move to 'branches'.
The code for retrieving Git branches is based on work by Dominik Ruf.
Up to commit 85d812ab4c64, the api 'pull' method can either pull from the repository's specified remote (clone_uri) or, if the repository is a fork, from the fork origin.
The existing test case only covered pulling from a remote, and pulling from a repo (non-fork) that does not have a remote specified.
Update the test method naming to clarify this, and add a test to pull from a fork.
tests: notification: clarify that cs_target_repo is a URL
In analogy with the fake URLs used for pullrequest notification testing, use an URL for cs_target_repo. When testing the resulting HTML in a real email client, it will become obvious that the mail client transforms URLs to something clickable, regardless of any present <a> tag.
The rendering of HTML emails in Outlook is very restricted and based on old Internet Explorer versions (more or less the same rendering as in Microsoft Word). To make the Kallithea emails look the same as in the browser, some adaptations are needed.
HTML email templates: Outlook: fix width and height attributes to preserve whitespace
The rendering of HTML emails in Outlook is very restricted and based on old Internet Explorer versions (more or less the same rendering as in Microsoft Word). To make the Kallithea emails look the same as in the browser, some adaptations are needed.
HTML width and height attributes do not have any effect, the size is interpreted as 0px. A CSS-based width and height do work in Outlook and result in the expected spacing between and within elements of the email.
For Outlook rendering, the CSS variant is probably sufficient, but as it is unclear how other mail agents will interpret this, leave the HTML width and height attributes as well.
HTML email templates: Outlook: fix padding inside of action button
The rendering of HTML emails in Outlook is very restricted and based on old Internet Explorer versions (more or less the same rendering as in Microsoft Word). To make the Kallithea emails look the same as in the browser, some adaptations are needed.
The padding inside the button is completely lost in Outlook, the text sticks to the border. This can be fixed by moving the padding styling from the <center> tag to the <td> cell.
HTML email templates: Outlook: center action button
The rendering of HTML emails in Outlook is very restricted and based on old Internet Explorer versions (more or less the same rendering as in Microsoft Word). To make the Kallithea emails look the same as in the browser, some adaptations are needed.
One of the problems is that the action button is not centered. An extra <center> tag solves this issue.
This removes the deprecation warning at the end of the test run: pytest-catchlog plugin has been merged into the core, please remove it from your requirements.
home: make sure users and group autocomplete is case insensitive
Both SQLite and MySQL have a case-insensitive LIKE operator by default, but PostgreSQL does not. As a result, a query for 'john' does not match the user 'John Doe'.
As case-insensitivity is most user-friendly in the context of autocompletion of users and groups, switch to the ilike() method of SQLAlchemy rather than like().
templates: mark site as IE10+ compatible to bypass Compatibility mode
Internet Explorer 11 (and probably other versions) seems to ship with the setting "Display intranet sites in Compatibility View" enabled by default. This compatibility view means that modern elements of CSS and JavaScript are not (well) support and makes Kallithea look very bad and become unusable.
This is a problem when hosting Kallithea in a corporate environment where its URL is detected as 'the intranet'.
Solve the issue by explicitly marking Kallithea to be compatible with IE10, so that Internet Explorer 10 and above will show Kallithea correctly in Intranet environments.
If no recipients are set, emails are sent to all admins and the address specified in the config file setting 'email_to'.
However, the existing code did not cater for the fact that multiple addresses could be set there: the setting was treated as a single string and passed along as-is.
Adapt the code to split the email_to contents on a comma, as this is the separator expected by backlash.
ini: error email settings are comma-separated under backlash
Document how it email_to works under backlash.
When specifying multiple email addresses to receive error emails, they should be comma-separated under backlash, rather than space-separated previously. Extra spaces should not be a problem but is not "supported"/"recommended".
- Updated Git repository implementation to ensure context falls within 0 to 2**31-1 range (inclusive) when fetching a diff. - Added tests for Git repositories for checking passed-in negative and overflowing contexts (for the --unified option). - Updated Mercurial repository implementation to ensure context is not negative when fetching a diff. - Added tests for Mercurial repositories for checking passed-in negative context (for the --unified option).
Dominik Ruf found that aa25ef34ebab introduced a regression in anonymous access to repositories ... if that is enabled.
The refactoring was too strict when it missed that not all repo permission checks require a logged in user. Read access can be granted to the default user ... but not write or admin.
Instead of the commands used in aa25ef34ebab, the following commands are used to consistently also allow the default user in all decorators where we only need repo read access:
# Introduce explicit allow_default_user=True - that was the default before aa25ef34ebab sed -i 's/@LoginRequired()/@LoginRequired(allow_default_user=True)/g' `hg mani` sed -i 's/@LoginRequired(\(..*\))/@LoginRequired(\1, allow_default_user=True)/g' `hg mani` # The primary case: Replace @NotAnonymous with removal of allow_default_user=True perl -0pi -e 's/\@LoginRequired\((?:(.*), )?allow_default_user=True\)\n\s*\@NotAnonymous\(\)/\@LoginRequired(\1)/g' `hg mani` # If there is a global permission check, no anonymous is ever allowed perl -0pi -e 's/\@LoginRequired\(allow_default_user=True\)(\n\s*\@HasPermission)/\@LoginRequired()\1/g' `hg mani` # Repo access for write or admin also assume no default user perl -0pi -e 's/\@LoginRequired\(allow_default_user=True\)(\n\s*\@HasRepoPermissionLevelDecorator\('"'(write|admin)'"'\))/\@LoginRequired()\1/g' `hg mani`
setup: updated dependencies to accept latest Pylons release (1.0.3).
A couple of weeks ago the Pylons project has released version 1.0.3, removing the previously stable version 1.0.2 in the process. In turn, this would mean that new installation of Kallithea will end-up with older version of Pylons than what is actually available, and in particular a much older release than 1.0.2.
- use table-layout: fixed Since we already use a fixed width for the most cells, this makes it easier (faster) for the browser to render specially big tables. And it allows to use 'text-overflow: ellipsis'. The old ellipsis settings have no effect.
- use nowrap and 'text-overflow: ellipsis' for all cells A table is easier (faster) to read for a user if all the row have the same height. Therefore don't wrap lines and in case of overflow use ellipsis.
- remove unnecessary height Up until now the row was bigger then 31px so this had no effect. And it look wrong to make the row bigger then its content.
- remove unnecessary border-color bootstrap default is almost the same color
- remove unnecessary text-align left is the default
- remove font-size or use bootstrap .small Making fonts too small hurts the readability.
- remove some padding and margin
- don't use special colors The differences are hardly noticable or in case of the gray dates hurt readability.
less: we should be consistent with diffstat-ish colors
With this change, on the changeset page, the 'change counters' for the whole changeset (in the top right corner) use the same colors as the counters for the lines (a little bit below) and the line change highlighting.
- move .changed from kallithea.less to kallithea-diff.less - be more generic with selectors in kallithea-diff - remove remaining style from kallithea.less
less: cleanup nav-pills menu style used in settings
- remove obsolete padding for li - remove style for 'li > a' that is aready set by bootstrap via .nav-pills - use usual color: inherit only for non active elements - override nav-pills-active-link-hover-color and nav-pills-active-link-hover-bg instead custom style
- use official dataTables.bootstrap.css and dataTables.bootstrap.js - add datatable less style block to use kallithea font icons - move dataTables related style to this new block to have them in one place - remove style that is obsolete, overrides or gets overriden by dataTables.bootstrap.css
In the future we'll probably use it to manage more then just less/css stuff. So the less directory is the wrong place. The most common place is the root directory, so lets put it there. Also, this way the --prefix parameter for npm is no longer required.
Where the additional spacing is needed, we already have a padding-right, so these spaces are unnecessary. But more importantly they cause trouble if text-decoration is used (i.e. underline on hover).
auth: refactor to introduce @LoginRequired(allow_default_user=True) and deprecate @NotAnonymous()
It was error prone that @LoginRequired defaulted to allow anonymous users (if 'default' user is enabled). See also 245b4e3abf39.
Refactor code to make it more explicit and safe by default: Deprecate @NotAnonymous by making it the default of @LoginRequired. That will make it safe by default.
To preserve same functionality, set allow_default_user=True in all the cases where @LoginRequired was *not* followed by @NotAnonymous or other permission checks - that was done with some script hacks: sed -i 's/@LoginRequired(\(..*\))/@LoginRequired(\1, allow_default_user=True)/g' `hg mani` sed -i 's/@LoginRequired()/@LoginRequired(allow_default_user=True)/g' `hg mani` perl -0pi -e 's/\@LoginRequired\(allow_default_user=True\)\n\s*\@NotAnonymous\(\)/\@LoginRequired()/g' `hg mani` perl -0pi -e 's/\@LoginRequired\(allow_default_user=True\)(\n\s*\@Has(Repo)?Permission)/\@LoginRequired()\1/g' `hg mani`
It has been reviewed that all uses of allow_default_user=True are in places where the there indeed wasn't any checking for default user before. These may or may not be correct, but now they are explicit and can be spotted and fixed.
The few remaining uses of @NotAnonymous should probably be removed somehow.
css: use pseudo-content trick to prevent diff line numbers from being pasted to text
When copy-pasting a diff from Chrome to a text editor, line numbers (on separate lines) would be pasted as well. Even though 'user-select: none' prevents text from being visually selected, in Chrome, the text still gets copied to the clipboard when the user for example presses ctrl-c. (It worked in Firefox.)
Instead, don't put the line numbers directly in the DOM, but put them in a data attribute and render them as :before. That will give the same rendering as before but prevent it from being copied.
(Firefox will however still add empty lines - that is how <pre> is hardcoded to be rendered when pasting to text.)
setup-db: print completion message to avoid confusion (issue #303)
There are cases where the last message of setup-db is a warning, giving the impression that the overall command failed which may not actually be the case.
For example, when git is not installed, warnings are given, but they are not an actual error. Kallithea will work fine for Mercurial repositories.
To avoid any confusion, print a completion message at the end. Any real errors will abort the command and not make it this far.