Once upon a time, in 1995, there were browsers that didn't understand <script>
tags and they would simply show the code inside as text. This started a
tradition of wrapping everything inside <script> in <!-- HTML comments -->.
Nowadays, it's not only not needed, but can be considered harmful[1]:
- within XHTML documents, the source will actually be hidden from all browsers
and rendered useless
- `--` is not allowed within HTML comments, so any decrement operations in
script are invalid
[1]: http://www.javascripttoolbox.com/bestpractices/#comments
Previously, when user scrolled down to see the next page on /graph, all hgweb
did was re-render everything that would be visible (by simply incrementing
revcount). It was not efficient at all, and this patch makes /graph page behave
similarly to the regular /log: every new page only consists of new changesets,
no duplication, and only jsdata is based on the full set of changesets required
to build accurate graph.
This is achieved by adding "?graphtop=<node>" to the next page URL template,
effectively remembering where the graph started, and using that value to create
the new `tree` that covers the whole visible graph. That variable is then used
to produce jsdata for redrawing graph client-side.
nextentry is used for the same purpose as on /log page (to format the next page
URL), but it's not a part of the graph.
hgweb determines and passes to templates some variables related to graph
appearance, like bg_height, canvaswidth and canvasheight. bg_height was and
still is used for graph.scale() call in graph.tmpl, and the two latter
variables were used in <canvas> element as width and height properties, and
they were set before JS code got to run. Setting these properties server-side
doesn't make a lot of sense, because a graph that has been scaled should
calculate things like width and height on its own when being rendered.
Let's move (re)sizing <canvas> to JavaScript (to Graph.render function) and
stop parsing HTML with regular expressions just to know new width and height.
That extra loop that only counts cols is required because <canvas> can't
be resized after or in the process of rendering (or it gets cleared).
Incidentally, SVG doesn't have this problem and I'm hoping to switch graph to
using it in future.
There also was truecanvasheight, but according to hg grep --all it was never
used, see f5506d2a674c.
Just to give some context to the return values: vertex() needs to return two
HTML elements as strings, <li> to be used as a background and a <li> to be
shown in foreground. The latter was made obsolete recently when changesets
started to be rendered server-side, but background elements are still useful
for now.
Historically, client-side graph code was not only rendering the graph itself,
but it was also adding all of the changeset information to the page as well.
It meant that JavaScript code needed to construct valid HTML as a string
(although proper escaping was done server-side). It wasn't too clunky, even
though it meant that a lot of server-side things were duplicated client-side
for no good reason, but the worst thing about it was the data format it used.
It was somewhat future-proof, but not human-friendly, because it was just a
tuple: it was possible to append things to it (as was done in e.g.
4d7cfa1867b5), but you'd then have to remember the indices and reading the
resulting JS code wasn't easy, because cur[8] is not descriptive at all.
So what would need to happen for graph to have more features, such as more
changeset information or a different vertex style (branch-closing, obsolete)?
First you'd need to take some property, process it (e.g. escape and pass
through templatefilters function, and mind the encoding too), append it to
jsdata and remember its index, then go add nearly identical JavaScript code to
4 different hgweb themes that use jsdata to render HTML, and finally try and
forget how brittle it all felt. Oh yeah, and the indices go to double digits if
we add 2 more items, say phase and obsolescence, and there are more to come.
Rendering vertex in a different style would need another property (say,
character "o", "_", or "x"), except if you want to be backwards-compatible, it
would need to go after tags and bookmarks, and that just doesn't feel right.
So here I'm trying to fix both the duplication of code and the data format:
- changesets will be rendered by hgweb templates the same way as changelog and
other such pages, so jsdata won't need any information that's not needed for
rendering the graph itself
- jsdata will be a dict, or an Object in JS, which is a lot nicer to humans and
is a lot more future-proof in the long run, because it doesn't use numeric
indices
What about hgweb themes? Obviously, this will break all hgweb themes that
render graph in JavaScript, including 3rd-party custom ones. But this will also
reduce the size of client-side code and make it more uniform, so that it can be
shared across hgweb themes, further reducing its size. The next few patches
demonstrate that it's not hard to adapt a theme to these changes. And in a
later series, I'm planning to move duplicate JS code from */graph.tmpl to
mercurial.js and leave only 4 lines of code embedded in those <script>
elements, and even that would be just to allow redefining graph.vertex
function. So adapting a custom 3rd-party theme to these changes would mean:
- creating or copying graphnode.tmpl and adding it to the map file (if a theme
doesn't already use __base__)
- modifying one line in graph.tmpl and simply removing the bigger part of
JavaScript code from there
Making these changes in this patch and not updating every hgweb theme that uses
jsdata at the same time is a bit of a cheat to make this series more
manageable: /graph pages that use jsdata are broken by this patch, but since
there are no tests that would detect this, bisect works fine; and themes are
updated separately, in the next 4 patches of this series to ease reviewing.
ajaxScrollInit is a function that loads more elements (e.g. changelog entries)
when browser window is scrolled down to the bottom of the page. It basically
fetches the next page from the server as HTML, finds container element in that
document and "adopts" (essentially, moves) all its child nodes to the container
in the current document.
Currently, hgweb doesn't render any changesets on /graph page (everything is
done in JavaScript), so there are no children to adopt. But there will be soon,
so let's create a reusable function that does it.
Hardcoding #graphnodes selector is suboptimal, but graph code already does this
in two other places.
"top: -Xpx" shifts a block up by X pixels, which can be used to visually
compress two lines of text to have less space between them, in this case it's
used for the changesets on /graph page. But not only it's not needed there
(both lines fit fine into their allowed vertical space), but it would also look
better (not as crammed, more vertically centered) without these negative
values.
"position: relative" is needed solely for the "top" property to have effect on
the element, no children of the .info element rely on it, so let's remove it as
well.
Unfortunately, current version of jshint (2.9.5) doesn't know such a global
variable and complains that it's undefined. Since this line tries to look up
URLSearchParams in a global scope (i.e. window), let's simply preface it with
"window." to work around jshint.
Variables that are used or assigned without any declaration using var (or let,
or const) are considered global. In many cases this is inadvertent and actually
causes a variable leaking to a broader scope, such as a temporary variable used
inside a loop suddenly being accessible in global scope. (This corresponds to
"undef" option of jshint).
So this patch limits the scope of variables that don't need to be global. There
are a lot of helper variables in Graph.render() used in a loop, I've declared
them all on one line to reduce patch size. "radius" is special because it
wasn't passed to graph.vertex, but was used there (it worked because this
variable leaked to global scope). "window.graph" is created by an inline script
in graph.tmpl so that it can be used in ajaxScrollInit() function, this patch
makes this fact explicit by assigning window.graph to a local variable.
"xhr" is a really widespread name for this kind of things, no idea where did
"xfr" come from (the original fbf9645839e4 doesn't explain that). Let's just
change one letter so the name makes more sense.
In JavaScript, using for-in loops to access every property of an object can
have unexpected results when inheritance is involved. For example, if some
piece of code adds a property (it may be a method too) to Object.prototype,
then all for-in loops that iterate over keys of any object (also anything that
inherits Object) will get that property on one of the iterations. To filter out
such unexpected properties for-in loops have to use Object.hasOwnProperty()
method. (This corresponds to "forin" option of jshint).
In the two first cases "data" and "edges" are arrays, to it's simpler to just
switch to using a regular for-with-a-counter loop.
This patch changes "==" (equals operator) to "===" (strict equals operator).
The difference between them is that the latter doesn't do any type coercions.
It's handy to compare string '1' to number 1 sometimes, but most of the time
using "==" is inadvertent and can be replaced by an explicit type conversion.
(This corresponds to "eqeqeq" option of jshint).
Some of the changes in this patch are straightforward, e.g. when comparing
results of typeof (they could only be strings). The same goes for 'none' and
similar strings that can't be sensibly coerced to some other type. Two changes
that compare values to "1" and "0" can be clarified: getAttribute() returns
either a string or null, but comparing null to a string is always false, so no
logic is lost.
The first hunk had a non-breaking space character just before "{", it's not an
error or anything, but let's fix it while we're at it. (This corresponds to
"nonbsp" option of jshint).
Hunks 2 and 3 change "==" (equals operator) to "===" (strict equals operator).
The difference between them is that the latter doesn't do any type coercions.
It's handy to compare string '1' to number 1 sometimes, but most of the time
using "==" is inadvertent and can be replaced by an explicit type conversion.
(This corresponds to "eqeqeq" option of jshint).
Most of this file already uses strict equals operator, and in the code affected
type coercion is not needed, because tagName and selectableTag are both strings
and endId and startId are both numbers.
mercurial.js has a process_dates() function that calculates relative age for a
given date, it works for all elements with "age" css class. If those elements
also have "date" css class, the original text is preserved and age is added at
the end.
This patch adds these two css classes in some pages in gitweb and monoblue that
weren't already using this feature.
Clicking on a line link on pages that show any kind of file contents (including
diffs) should highlight that line, and in monoblue it works when there's a
<pre> element (e.g. diff), but pages that use <table> element (annotate and
compare) need this css class. It matches and highlights linked (":target")
table rows. This line is pretty much copied from gitweb theme.
There's a visual difference in hgweb between one changeset that is the tip of
its branch and another that simply belongs to that branch. But paper theme
ignored this difference on changeset page and used to always use "branchname"
css class, be that changeset the tip of its branch or not. That has been
recently fixed, so this piece of css is not needed anymore.
Let's make "instabilities" list contain items with "instability" key as opposed
to "name" key. This way it's more explicit and more consistent with the log
command-line template.
This makes graphdata() simpler by using existing code that gets common
changeset properties for showing in hgweb. graphdata() is a nested function in
graph() that prepares entries for /graph view, but there are two different
lists of changesets prepared: "jsdata" for JavaScript-rendered graph and
"nodes" for everything else.
For "jsdata", properties "node", "user", "age" and "desc" are passed through
various template filters because we don't have these filters in JavaScript, so
the data has to be prepared server-side. But now that commonentry() is used for
producing "nodes" list (and it doesn't apply any filters), these filters need
to be added to the appropriate templates (only raw at this moment, everything
else either doesn't implement graph or uses JavaScript).
This is a bit of refactoring that will hopefully simplify future patches. The
end result is to have /graph that only renders the actual graph with nodes and
vertices in JavaScript, and the rest is done server-side. This way server-side
code can focus on showing a list of changesets, which is easy because we
already have /log, /shortlog, etc, and JavaScript code can be simplified,
making it easier to add obsolescence graph and other features.
In paper, coal, gitweb and monoblue a new "tag" (or multiple, if there are many
instabilities) is added to the same line that has phase, branch, etc of a
changeset; in gitweb and monoblue this element has a light red background, in
paper and coal the element is black and underlined. In spartan theme
instabilities are shown on a separate line.
While test-obsolete.t uses first(phasedivergent()) revset to pick a changeset
to test, that particular changeset is also an orphan, so two different
instability tags are displayed.
This anchor text is problematic in two ways: first, the "back to" part
assumes that you got to the page showing it via the filelog. This is
not necessarily true, as there are other ways to get to that view
besides the filelog view, such as for example following the history of
lines from a file. Second, it uses "filelog" jargon, which refers to
how each file has its own revlog. This is internal jargon that has no
business being exposed to the end user.
I just reworded this template to improve understanding.
As with phases, spartan theme shows a simple "obsolete: yes" on its own line
(this allows replacing "yes" with something more useful in future, like output
of obsfate* template functions). Everywhere else a new "tag" is added to the
same line that has phase, branch, etc of a changeset; in gitweb and monoblue
the element has gray background, in paper and coal the element is gray with a
dashed underline.
In spartan theme phase is shown on its own table row, because there's no single
line of "tags". Everywhere else phase is prepended to the list of "tags" of a
changeset. Its element has a purple-ish color in gitweb and monoblue, and a
dotted line under it and no color in paper and coal (as these themes are frugal
with colors).
This patch intentionally doesn't touch graph, because it needs a rewrite. I'll
get to it pretty soon and in the process will add phase and everything that's
still coming (e.g. obsolescence and instabilities).
.. feature::
hgweb now displays phases of non-public changesets
This patch puts all these changeset "tags" into one template shared everywhere
in paper and coal themes. But it should be noted that some of the templates had
different sets of tags, in some cases it was intended, in others - most likely
not.
First, what's up with all these different ways to get changeset's branch. There
are actually 3 ways to do it in hgweb, they can all be seen in this patch;
"branches", "inbranch" and "branch". They are all lists that consist of 1 or 0
items:
- "branches" has ctx.branch() if current changeset is the tip of that branch
- "inbranch" has ctx.branch() if current changeset is _not_ the tip of that
branch and the branch is not "default"
- "branch" aka "changesetbranch" has ctx.branch() if the branch is not
"default"
The majority of cases (7 vs 2 + /graph) in paper theme used only option 3,
which meant that "default" was never displayed. But other parts of the theme
disagreed with this and used option 1 and option 2 together. For example, the
default view (log) displays "default" on the branch tip (can be seen right
about now on m-s.o/repo/hg), but it disappears when you click on the commit.
Also, using option 3 alone meant that there was no way to tell if a changeset
is the tip of its branch or not (it was always assumed that it's not, see how
some css classes change from "branchname" to the correct "branchhead" in tests)
-- so the two different css styles that exist in paper just for this were
underused.
I think this patch improves the situation, even though it changes the old (even
if inconsistent) behavior. The new behavior matches that of gitweb and
monoblue.
Less duplication and it's also easier to add extra "tags" everywhere at once.
These aren't tags as defined `hg help glossary` (hence the quotes), they are
simply called that. They include branch name (in different styles if changeset
is a head of that branch or not), (actual) tags and bookmarks. Good candidates
to add to this list would be changeset phase and obsoletion status.
Everything is pretty self-explanatory except the last hunk, where jshint
complains: "Misleading line break before '||'; readers may interpret this as an
expression boundary."
There is a tweakable called "laxbreak" that allows line breaks before
operators, but I think it's fine to simply join this one line and avoid extra
config for now (we can cook up and add a sensible .jshintrc later).
These new colors and styles are already used in the tooltip that gets shown
when user hovers over line numbers on annotate page. The old styles, replaced
in this patch, look completely unrelated to gitweb or any other hgweb theme.
These new colors and styles are already used in the tooltip that gets shown
when user hovers over line numbers on annotate page. The old styles, replaced
in this patch, look completely unrelated to paper or any other hgweb theme.
Use the verbosity aware template keyword introduced earlier. It has the nice
property of being verbosity dependent but in order to customize the obsfate
part, users will need to replace the lobsfate definition from default mapfile
with the one using template functions (by copying the one from test-obsmarker-
template.t for example).
As it's a more advanced use-case, I'm more inclined to have the same code for
the {obsfate} keyword, in the changeset printer and in the default mapfile for
consistency.
But, the definition in default mapfile could be replaced with one based on
template filter to obsfate output customization if it is a big need for users.
This allows us to %include map-cmdline.<style> file in our .hgrc files. The
syntax is slightly different as hgrc doesn't support loading an external
template file, but map-cmdline files don't use this feature, so the syntax
can be considered identical in practice.
Unnamed section is remapped for backward compatibility.
Building on top of the new URL query string arguments to control
whitespace settings for annotate, this commit adds HTML checkboxes
reflecting the values of these arguments to the paper and gitweb
themes.
The actual diff settings are now exported to the templating layer.
The HTML templates add these as data-* attributes so they are
accessible to the DOM.
A new <form> with various <input> elements is added. The <form>
is initially hidden via CSS. A shared JavaScript function (which
runs after the <form> has been rendered but before the annotate
HTML (because annotate HTML could take a while to load and we want
the form to render quickly) takes care of setting the checked state
of each box from the data-* attributes. It also registers an event
handler to modify the URL and refresh the page whenever the checkbox
state is changed.
I'm using the URLSearchParams interface to perform URL manipulation.
https://developer.mozilla.org/en-US/docs/Web/API/URLSearchParams tells
me this may not be supported on older web browsers. Yes, apparently
the web API didn't have a standard API to parse and format query
strings until recently. Hence the check for the presence of this
feature in the JavaScript. If the browser doesn't support the
feature, the <form> will remain hidden and behavior will like it
currently is. We could polyfill this feature or implement our own
query string parsing. But I'm lazy and this could be done as a
follow-up if people miss it.
We could certainly expand this feature to support more diff options
(such as lines of context). That's why the potentially reusable code
is stored in a reusable place. It is also certainly possible to
add diff controls to other pages that display diffs. But since
Mozillians are making noise about controlling which revisions
annotate shows, I figured I'd start there.
.. feature::
Control whitespace settings for annotation on hgweb
/annotate URLs on hgweb now accept query string arguments to
influence how whitespace changes impact results.
The arguments "ignorews," "ignorewsamount," "ignorewseol," and
"ignoreblanklines" now have the same meaning as their [annotate]
config section counterparts. Any provided setting overrides the
server default.
HTML checkboxes have been added to the paper and gitweb themes
to expose current whitespace settings and to easily modify the
current view.
Differential Revision: https://phab.mercurial-scm.org/D850
