Patchwork [4,of,4] hgweb: remove now unnecessary explicit header() and footer()

login
register
mail settings
Submitter Alexander Plavin
Date Aug. 2, 2013, 9:39 p.m.
Message ID <11ac049356d489464231.1375479555@debian-alexander.dolgopa>
Download mbox | patch
Permalink /patch/1993/
State Superseded
Commit 1dba26575dba49be8d7d2e6589c227419cfd1895
Headers show

Comments

Alexander Plavin - Aug. 2, 2013, 9:39 p.m.
# HG changeset patch
# User Alexander Plavin <alexander@plav.in>
# Date 1374621626 -14400
#      Wed Jul 24 03:20:26 2013 +0400
# Node ID 11ac049356d4894642313803d9edc1ff2e9d2788
# Parent  1fe956e499f2996d4b63ae04c2ec99fe39e89302
hgweb: remove now unnecessary explicit header() and footer()
Martin Geisler - Aug. 9, 2013, 7:07 a.m.
Alexander Plavin <alexander@plav.in> writes:

> # HG changeset patch
> # User Alexander Plavin <alexander@plav.in>
> # Date 1374621626 -14400
> #      Wed Jul 24 03:20:26 2013 +0400
> # Node ID 11ac049356d4894642313803d9edc1ff2e9d2788
> # Parent  1fe956e499f2996d4b63ae04c2ec99fe39e89302
> hgweb: remove now unnecessary explicit header() and footer()

I think it could be nice if you added a tiny bit of context: when and
why did this become unnecessary (which changeset does "now" refer to)?

My first guess would be that the immediate parent (patch 3) makes the
functions unnecessary, but it doesn't look like it to me. So my second
guess is that "now" refers to patch 2. That suggests that the patches
could be reordered by swapping 3 and 4.
Alexander Plavin - Aug. 9, 2013, 9:11 a.m.
09.08.2013, 11:07, "Martin Geisler" <martin@geisler.net>:
> Alexander Plavin <alexander@plav.in> writes:
>
>>  # HG changeset patch
>>  # User Alexander Plavin <alexander@plav.in>
>>  # Date 1374621626 -14400
>>  #      Wed Jul 24 03:20:26 2013 +0400
>>  # Node ID 11ac049356d4894642313803d9edc1ff2e9d2788
>>  # Parent  1fe956e499f2996d4b63ae04c2ec99fe39e89302
>>  hgweb: remove now unnecessary explicit header() and footer()
>
> I think it could be nice if you added a tiny bit of context: when and
> why did this become unnecessary (which changeset does "now" refer to)?
>
> My first guess would be that the immediate parent (patch 3) makes the
> functions unnecessary, but it doesn't look like it to me. So my second
> guess is that "now" refers to patch 2. That suggests that the patches
> could be reordered by swapping 3 and 4.

You are right that this depends on patch 2 only, but why reorder? Third patch also depends on that, so it could cause the same question as well if they were reordered :)

>
> --
> Martin Geisler
Martin Geisler - Aug. 9, 2013, 11:30 a.m.
Alexander Plavin <alexander@plav.in> writes:

> 09.08.2013, 11:07, "Martin Geisler" <martin@geisler.net>:
>> Alexander Plavin <alexander@plav.in> writes:
>>
>>>  # HG changeset patch
>>>  # User Alexander Plavin <alexander@plav.in>
>>>  # Date 1374621626 -14400
>>>  #      Wed Jul 24 03:20:26 2013 +0400
>>>  # Node ID 11ac049356d4894642313803d9edc1ff2e9d2788
>>>  # Parent  1fe956e499f2996d4b63ae04c2ec99fe39e89302
>>>  hgweb: remove now unnecessary explicit header() and footer()
>>
>> I think it could be nice if you added a tiny bit of context: when and
>> why did this become unnecessary (which changeset does "now" refer to)?
>>
>> My first guess would be that the immediate parent (patch 3) makes the
>> functions unnecessary, but it doesn't look like it to me. So my second
>> guess is that "now" refers to patch 2. That suggests that the patches
>> could be reordered by swapping 3 and 4.
>
> You are right that this depends on patch 2 only, but why reorder?
> Third patch also depends on that, so it could cause the same question
> as well if they were reordered :)

Aha... :-) I somehow thought that moving a string as you do in patch 3
was already supported. Maybe you could explain more clearly in patch 2
that it allows you to use the map file as a key-value store for plain
strings as well (if that is actually what patch 2 does).
Alexander Plavin - Aug. 9, 2013, 11:52 a.m.
09.08.2013, 15:30, "Martin Geisler" <martin@geisler.net>:
> Alexander Plavin <alexander@plav.in> writes:
>
>>  09.08.2013, 11:07, "Martin Geisler" <martin@geisler.net>:
>>>  Alexander Plavin <alexander@plav.in> writes:
>>>>   # HG changeset patch
>>>>   # User Alexander Plavin <alexander@plav.in>
>>>>   # Date 1374621626 -14400
>>>>   #      Wed Jul 24 03:20:26 2013 +0400
>>>>   # Node ID 11ac049356d4894642313803d9edc1ff2e9d2788
>>>>   # Parent  1fe956e499f2996d4b63ae04c2ec99fe39e89302
>>>>   hgweb: remove now unnecessary explicit header() and footer()
>>>  I think it could be nice if you added a tiny bit of context: when and
>>>  why did this become unnecessary (which changeset does "now" refer to)?
>>>
>>>  My first guess would be that the immediate parent (patch 3) makes the
>>>  functions unnecessary, but it doesn't look like it to me. So my second
>>>  guess is that "now" refers to patch 2. That suggests that the patches
>>>  could be reordered by swapping 3 and 4.
>>  You are right that this depends on patch 2 only, but why reorder?
>>  Third patch also depends on that, so it could cause the same question
>>  as well if they were reordered :)
>
> Aha... :-) I somehow thought that moving a string as you do in patch 3
> was already supported. Maybe you could explain more clearly in patch 2
> that it allows you to use the map file as a key-value store for plain
> strings as well (if that is actually what patch 2 does).

I've thought a bit about better commit message for 2nd patch now, but couldn't improve what's already there :) Quote from that commit: "allows adding arbitrarily-named entries to a template map file, and then referencing them" - this includes plain strings too. Any suggestion?

>
> --
> Martin Geisler
Martin Geisler - Aug. 9, 2013, 12:30 p.m.
Alexander Plavin <alexander@plav.in> writes:

> 09.08.2013, 15:30, "Martin Geisler" <martin@geisler.net>:
>> Alexander Plavin <alexander@plav.in> writes:
>>
>>>  09.08.2013, 11:07, "Martin Geisler" <martin@geisler.net>:
>>>>  Alexander Plavin <alexander@plav.in> writes:
>>>>>   # HG changeset patch
>>>>>   # User Alexander Plavin <alexander@plav.in>
>>>>>   # Date 1374621626 -14400
>>>>>   #      Wed Jul 24 03:20:26 2013 +0400
>>>>>   # Node ID 11ac049356d4894642313803d9edc1ff2e9d2788
>>>>>   # Parent  1fe956e499f2996d4b63ae04c2ec99fe39e89302
>>>>>   hgweb: remove now unnecessary explicit header() and footer()
>>>>  I think it could be nice if you added a tiny bit of context: when and
>>>>  why did this become unnecessary (which changeset does "now" refer to)?
>>>>
>>>>  My first guess would be that the immediate parent (patch 3) makes the
>>>>  functions unnecessary, but it doesn't look like it to me. So my second
>>>>  guess is that "now" refers to patch 2. That suggests that the patches
>>>>  could be reordered by swapping 3 and 4.
>>>  You are right that this depends on patch 2 only, but why reorder?
>>>  Third patch also depends on that, so it could cause the same question
>>>  as well if they were reordered :)
>>
>> Aha... :-) I somehow thought that moving a string as you do in patch 3
>> was already supported. Maybe you could explain more clearly in patch 2
>> that it allows you to use the map file as a key-value store for plain
>> strings as well (if that is actually what patch 2 does).
>
> I've thought a bit about better commit message for 2nd patch now, but
> couldn't improve what's already there :) Quote from that commit:
> "allows adding arbitrarily-named entries to a template map file, and
> then referencing them" - this includes plain strings too. Any
> suggestion?

I guess the problem is that I don't know what an "arbitrarily-named
entry" is :)

It's been a long time since I looked at the map file format in detail,
but I were asked to describe it, I would describe the format as a bunch of

  key = value

lines where "value" can reference other keys. If I would have to name
one of sides "entry", I would have described the lines this:

  entry = expansion

I now get the impression that you would describe the format as:

  reference = entry

or something similar.
Alexander Plavin - Aug. 9, 2013, 12:46 p.m.
09.08.2013, 16:30, "Martin Geisler" <martin@geisler.net>:
> Alexander Plavin <alexander@plav.in> writes:
>
>>  09.08.2013, 15:30, "Martin Geisler" <martin@geisler.net>:
>>>  Alexander Plavin <alexander@plav.in> writes:
>>>>   09.08.2013, 11:07, "Martin Geisler" <martin@geisler.net>:
>>>>>   Alexander Plavin <alexander@plav.in> writes:
>>>>>>    # HG changeset patch
>>>>>>    # User Alexander Plavin <alexander@plav.in>
>>>>>>    # Date 1374621626 -14400
>>>>>>    #      Wed Jul 24 03:20:26 2013 +0400
>>>>>>    # Node ID 11ac049356d4894642313803d9edc1ff2e9d2788
>>>>>>    # Parent  1fe956e499f2996d4b63ae04c2ec99fe39e89302
>>>>>>    hgweb: remove now unnecessary explicit header() and footer()
>>>>>   I think it could be nice if you added a tiny bit of context: when and
>>>>>   why did this become unnecessary (which changeset does "now" refer to)?
>>>>>
>>>>>   My first guess would be that the immediate parent (patch 3) makes the
>>>>>   functions unnecessary, but it doesn't look like it to me. So my second
>>>>>   guess is that "now" refers to patch 2. That suggests that the patches
>>>>>   could be reordered by swapping 3 and 4.
>>>>   You are right that this depends on patch 2 only, but why reorder?
>>>>   Third patch also depends on that, so it could cause the same question
>>>>   as well if they were reordered :)
>>>  Aha... :-) I somehow thought that moving a string as you do in patch 3
>>>  was already supported. Maybe you could explain more clearly in patch 2
>>>  that it allows you to use the map file as a key-value store for plain
>>>  strings as well (if that is actually what patch 2 does).
>>  I've thought a bit about better commit message for 2nd patch now, but
>>  couldn't improve what's already there :) Quote from that commit:
>>  "allows adding arbitrarily-named entries to a template map file, and
>>  then referencing them" - this includes plain strings too. Any
>>  suggestion?
>
> I guess the problem is that I don't know what an "arbitrarily-named
> entry" is :)
>
> It's been a long time since I looked at the map file format in detail,
> but I were asked to describe it, I would describe the format as a bunch of
>
>   key = value
>
> lines where "value" can reference other keys. If I would have to name
> one of sides "entry", I would have described the lines this:
>
>   entry = expansion
>
> I now get the impression that you would describe the format as:
>
>   reference = entry
>
> or something similar.

Yes, the format is 'key = entry', where by entry I mean entry content, expansion in your terms. And now (without the 2nd patch from this series) you can define entries with only hard-coded keys, not arbitrary.

>
> --
> Martin Geisler

Patch

diff -r 1fe956e499f2 -r 11ac049356d4 mercurial/hgweb/hgweb_mod.py
--- a/mercurial/hgweb/hgweb_mod.py	Thu Jul 25 01:12:25 2013 +0400
+++ b/mercurial/hgweb/hgweb_mod.py	Wed Jul 24 03:20:26 2013 +0400
@@ -331,12 +331,6 @@ 
 
         # some functions for the templater
 
-        def header(**map):
-            yield tmpl('header', encoding=encoding.encoding, **map)
-
-        def footer(**map):
-            yield tmpl("footer", **map)
-
         def motd(**map):
             yield self.config("web", "motd", "")
 
@@ -373,8 +367,7 @@ 
                                              "staticurl": staticurl,
                                              "urlbase": urlbase,
                                              "repo": self.reponame,
-                                             "header": header,
-                                             "footer": footer,
+                                             "encoding": encoding.encoding,
                                              "motd": motd,
                                              "sessionvars": sessionvars,
                                              "pathdef": makebreadcrumb(req.url),
diff -r 1fe956e499f2 -r 11ac049356d4 mercurial/hgweb/hgwebdir_mod.py
--- a/mercurial/hgweb/hgwebdir_mod.py	Thu Jul 25 01:12:25 2013 +0400
+++ b/mercurial/hgweb/hgwebdir_mod.py	Wed Jul 24 03:20:26 2013 +0400
@@ -408,12 +408,6 @@ 
 
     def templater(self, req):
 
-        def header(**map):
-            yield tmpl('header', encoding=encoding.encoding, **map)
-
-        def footer(**map):
-            yield tmpl("footer", **map)
-
         def motd(**map):
             if self.motd is not None:
                 yield self.motd
@@ -448,8 +442,7 @@ 
             staticurl += '/'
 
         tmpl = templater.templater(mapfile,
-                                   defaults={"header": header,
-                                             "footer": footer,
+                                   defaults={"encoding": encoding.encoding,
                                              "motd": motd,
                                              "url": url,
                                              "logourl": logourl,