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

[Angel Ezquerra]

On Thu, Feb 25, 2016 at 5:53 PM, Angel Ezquerra
<angel.ezquerra at gmail.com> wrote:
> 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?

Please scratch that. I just realized that I confused the "short_id"
with the "show_id" functions! :->

Cheers,

Angel