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

[Angel Ezquerra]

On Thu, Feb 25, 2016 at 2:38 PM, Mads Kiilerich <mads at kiilerich.com> wrote:
> On 02/24/2016 10:37 PM, Angel Ezquerra wrote:
>>>
>>> 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.
>
>
> 2: Considering the amount of code that is being called in the proposed
> scheme, I don't think it is simple. Can you make a full specification that
> can be implemented in some other programming language?

I can't really disagree. My proposal is not as simple as I would like.
What I would really like is to be able to provide the path, _as is_
(i.e. no encoding, other than perhaps normalizing the slashes) to
Kallithea.

> 3: The proposed scheme is "approximately readable" but generally not
> writable - and the details are not at all discoverable/guessable. And for
> 100 characters long file names with odd characters, it is hard to read /
> compare targets.
>
>> Your proposal covers well
>> #1,#2, #4 and #5 but does poorly at #3.
>
>
> 3: I would claim that someone who seriously wants to reverse engineer the
> scheme, sooner would get a fully correct implementation of the existing
> scheme than of the proposed one. Even more so if we document what the scheme
> is.
>
>> I'd rather get your proposal in than nothing,
>
>
> Byt "my proposal" is already in? What more is needed?

I think that I misunderstood you, and that I misunderstood the way the
FID is calculated. I thought that it took into account both the
current revision hash as well as the parent hash. It seems I was wrong
(although including the current revision hash on the file anchors is a
bit redundant).

Looking into the code I see that it uses the short_id function, which
can be configured to use different "SHA lenghts" and include the
revision number. Is there a reason why we would want to make it
possible to change the file anchors?

>> 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?
>
> They would guess it the same way as guessing that they have to use the
> Python library and apply some specific tweaks: Don't guess it! Search and
> find the answer. In the source, in google finding this discussion, in
> documentation ... or by asking.

I'd rather have an "obvious" solution, but I get your point.

>> 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?
>
>
> You can perhaps make the controller read the URL and put something into the
> page that make javascript navigate to that point ... or make javascript
> compute the target based on the URL.

I'm not sure how I would do that but it is worth exploring. For now it
seems that I can use what I learned to add this capability to
TortoiseHg without further changes to Kallithea.

Cheers,

Angel