[PATCH] changeset: add filename derived anchors to file diffs (and use them)

[Angel Ezquerra]

On Mon, Feb 22, 2016 at 9:42 PM, Mads Kiilerich <mads at kiilerich.com> wrote:
> On 02/21/2016 09:59 PM, Angel Ezquerra wrote:
>>
>> # HG changeset patch
>> # User Angel Ezquerra <angel.ezquerra at gmail.com>
>> # Date 1456088218 -3600
>> #      Sun Feb 21 21:56:58 2016 +0100
>> # Branch stable
>> # Node ID f1e465120793c9c7cad6baa4d78a00155bac4311
>> # Parent  f7b771cc04c39f5d49b7b82397bd594db7285693
>> changeset: add filename derived anchors to file diffs (and use them)
>>
>> Up until now it was not possible to create direct links to the file diffs
>> within
>> a changeset page because those links were created based on each file FID
>> (which
>> users cannot easily guess).
>>
>> This changeset adds anchors to each file diff. Each anchor name is derived
>> from
>> the corresponding filepath, which has been encoded to make it suitable to
>> be
>> used as URL anchor names as follows:
>>
>> 1. replace all forward slashes (/) with backward slashes (\)
>> 2. (python urlib) URL quoting
>> 3. replace all periods with exclamation marks
>>
>> This encoding is necessary (particularly the \ to / and . to ! mapping)
>> because
>> browsers have problems with anchors that contain forward slashes and
>> periods.
>>
>> Note that the original FID link targets have not been removed. Thus this
>> change
>> should be transparent to the rest of Kallithea.
>
>
> One feature of the existing FID is that it includes the revision hash and
> thus also generates unique ids for pages like
> https://kallithea-scm.org/repos/kallithea/changeset/494989b0ce72...b1bec568c0a2
> - it seems like your patch will make that URL generate (slightly) invalid
> html. That might be ok for the convenience this adds, but then that should
> be acknowledged explicitly.

Yes, apparently my patch generate some slightly invalid HTML. I was
not aware of it until Soren pointed it out. That being said I've
tested it with all major browsers and it does work fine with all of
them.

> The existing FID is defined in
> https://kallithea-scm.org/repos/kallithea/files/05a85a6cecba5c8caeb7996590365d5d9bc523c9/kallithea/lib/helpers.py#L123
> . It is not perfect but seems to me like it is significantly simpler than
> the alternative proposal. Especially because including "python urllib" in
> the specification sounds inconvenient for implementation in other languages
> and probably not is a place where Python guarantee 100% stability -
> especially not as various vulnerabilities are discovered.
>
> It _could_ perhaps make sense to define a special format for "I don't care
> about revision" links ... but why not just set it to "" like for PRs and get
> ids like "C--067e5b4403fb".
>
> So the current Python reference implementation for PR ids is:
> "C--" +
> hashlib.md5(u'kallithea/templates/changelog/changelog_summary_data.html'.encode('utf8')).hexdigest()[:12]
> - that must be easy to add to TortoiseHg.
>
> Or using my favourite scripting language:
> printf kallithea/templates/changelog/changelog_summary_data.html | md5sum
> |sed -ne 's,^\(.\{12\}\).*,C--\1,p'
>
> Can it be much simpler?
>
> (I guess one reason '.' generally should be avoided in ids is that it would
> be inconvenient when referencing it from css.)

The ideal solution to the problem I'm trying to solve would have 4 features:
1. A way to generate unique links to navigate directly to any file
diff within a changeset review page
2. Be simple
3. Be highly discoverable / guessable
4. Be stable
5. Generate legal HTML

My proposal covers #1, does OK (but not pefect by any means) with #2
and #3 and fails (slightly) #4 and #5. Your proposal covers well
#1,#2, #4 and #5 but does poorly at #3.
I'd rather get your proposal in than nothing, but I worry that
"non-insiders" will never be able to use the feature. How could they
guess the steps they should follow to generate the required HTML
address?

An ideal solution would be to be able to do something akin to what you
can do to access the raw-files in the regular mercurial server (and
Kallithea too, I think). With the regular server you can just append a
'raw-file' plus the file path to the changeset revision to get the
corresponding version of a the file on that changeset. It would be
neat to be able to do that somehow, and thus completely avoid the need
to encode the file path (beyond what urllib.pathname2url does, I
guess). Do you think there is some way to do that?

Cheers,

Angel