The "download mbox" file downloads the entire thread, which is rarely
what you want to do. Provide an alternative download link that only
includes the patches and, when the tags module is active, includes the
tags into the mbox.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
mods/tags.py | 47 +++++++++++++++++++++++++++++++-
www/templates/series-detail.html | 6 ++--
www/urls.py | 1 +
www/views.py | 11 ++++++++
4 files changed, 61 insertions(+), 4 deletions(-)
diff --git a/mods/tags.py b/mods/tags.py
index aa29c60..f7b0300 100644
--- a/mods/tags.py
+++ b/mods/tags.py
@@ -10,11 +10,13 @@
from mod import PatchewModule
from mbox import parse_address
+from django.http import HttpResponse, Http404
from event import register_handler, emit_event, declare_event
from api.models import Message
from api.rest import PluginMethodField
from patchew.tags import lines_iter
import re
+import www.views
REV_BY_PREFIX = "Reviewed-by:"
BASED_ON_PREFIX = "Based-on:"
@@ -27,6 +29,21 @@ tags = Tested-by, Reported-by, Acked-by, Suggested-by
BUILT_IN_TAGS = [REV_BY_PREFIX, BASED_ON_PREFIX]
+_instance = None
+
+# This is monkey-patched into www.views
+def _view_series_mbox_patches(request, project, message_id):
+ global _instance
+ s = Message.objects.find_series(message_id, project)
+ if not s:
+ raise Http404("Series not found")
+ if not s.is_complete:
+ raise Http404("Series not complete")
+ mbox = "\n".join(["From %s %s\n" % (x.get_sender_addr(), x.get_asctime()) + \
+ _instance.get_mbox_with_tags(x) for x in s.get_patches()])
+ return HttpResponse(mbox, content_type="text/plain")
+www.views.view_series_mbox_patches = _view_series_mbox_patches
+
class SeriesTagsModule(PatchewModule):
"""
@@ -49,6 +66,9 @@ series cover letter, patch mail body and their replies.
default_config = _default_config
def __init__(self):
+ global _instance
+ assert _instance == None
+ _instance = self
register_handler("MessageAdded", self.on_message_added)
declare_event("TagsUpdate", series="message object that is updated")
@@ -153,4 +173,29 @@ series cover letter, patch mail body and their replies.
"char": "O",
"row_class": "obsolete"
})
-
+ message.extra_links.append({"html": mark_safe(html), "icon": "exchange" })
+
+ # FIXME: what happens with base64 messages?
+ def get_mbox_with_tags(self, m):
+ def result_iter():
+ regex = self.get_tag_regex()
+ old_tags = set()
+ lines = lines_iter(m.get_mbox())
+ need_minusminusminus = False
+ for line in lines:
+ if line.startswith('---'):
+ need_minusminusminus = True
+ break
+ yield line
+ if regex.match(line):
+ old_tags.add(line)
+
+ # If no --- line, tags go at the end as there's no better place
+ for tag in m.get_property("tags", []):
+ if not tag in old_tags:
+ yield tag
+ if need_minusminusminus:
+ yield line
+ yield from lines
+
+ return '\n'.join(result_iter())
diff --git a/www/templates/series-detail.html b/www/templates/series-detail.html
index 52b8f96..1735794 100644
--- a/www/templates/series-detail.html
+++ b/www/templates/series-detail.html
@@ -26,9 +26,9 @@
{{ series.age }} ago</span>
<ul id="series-links">
<li>
- <span class="fa fa-download"></span> <a
- href="/{{ project }}/{{ series.message_id }}/mbox">Download series mbox</a></li>
- </li>
+ <span class="fa fa-download"></span>Download {% if series.is_complete %}mbox: <a
+ href="/{{ project }}/{{ series.message_id }}/patches.mbox">patches</a>, {% endif %}<a
+ href="/{{ project }}/{{ series.message_id }}/mbox">full thread</a></li>
{% for op in series.extra_links %}
<li>
<span class="fa fa-{% if op.icon %}{{ op.icon }}{% else %}question{% endif %}"></span>
diff --git a/www/urls.py b/www/urls.py
index 1659ff2..2087fec 100644
--- a/www/urls.py
+++ b/www/urls.py
@@ -35,5 +35,6 @@ urlpatterns += [
url(r"^(?P<project>[^/]*)/(?P<message_id>[^/]*)/$", views.view_series_detail, name="series_detail"),
url(r"^(?P<project>[^/]*)/(?P<thread_id>[^/]*)/(?P<message_id>[^/]*)/$", views.view_series_message, name="series_message"),
url(r"^(?P<project>[^/]*)/(?P<message_id>[^/]*)/mbox$", views.view_series_mbox),
+ url(r"^(?P<project>[^/]*)/(?P<message_id>[^/]*)/patches.mbox$", views.view_series_mbox_patches),
url(r"^$", views.view_project_list, name="project_list"),
]
diff --git a/www/views.py b/www/views.py
index d7c60dd..f2765fe 100644
--- a/www/views.py
+++ b/www/views.py
@@ -245,6 +245,17 @@ def view_series_mbox(request, project, message_id):
x.get_mbox() for x in r])
return HttpResponse(mbox, content_type="text/plain")
+def view_series_mbox_patches(request, project, message_id):
+ s = api.models.Message.objects.find_series(message_id, project)
+ if not s:
+ raise Http404("Series not found")
+ if not s.is_complete:
+ raise Http404("Series not complete")
+ mbox = "\n".join(["From %s %s\n" % (x.get_sender_addr(), x.get_asctime()) + \
+ x.get_mbox(x) for x in s.get_patches()])
+ return HttpResponse(mbox, content_type="text/plain")
+
+
def view_series_detail(request, project, message_id):
s = api.models.Message.objects.find_series(message_id, project)
if not s:
--
2.17.1
_______________________________________________
Patchew-devel mailing list
Patchew-devel@redhat.com
https://www.redhat.com/mailman/listinfo/patchew-devel
On Mon, 07/30 12:44, Paolo Bonzini wrote: > The "download mbox" file downloads the entire thread, which is rarely > what you want to do. Provide an alternative download link that only > includes the patches and, when the tags module is active, includes the > tags into the mbox. I say we keep the option simple and drop all reply messages. I don't think the entire thread is useful to anyone too. If they really want it they can switch to REST API or maybe we can implement a "thread reader" in patchew-cli. :) Fam _______________________________________________ Patchew-devel mailing list Patchew-devel@redhat.com https://www.redhat.com/mailman/listinfo/patchew-devel
On 02/08/2018 10:38, Fam Zheng wrote: >> The "download mbox" file downloads the entire thread, which is rarely >> what you want to do. Provide an alternative download link that only >> includes the patches and, when the tags module is active, includes the >> tags into the mbox. > I say we keep the option simple and drop all reply messages. I don't think the > entire thread is useful to anyone too. If they really want it they can switch to > REST API or maybe we can implement a "thread reader" in patchew-cli. :) Sounds good. There's still the question of what to do about quoted-printable or base64 patches, but I'll change the patches to drop the old code. Paolo _______________________________________________ Patchew-devel mailing list Patchew-devel@redhat.com https://www.redhat.com/mailman/listinfo/patchew-devel
On Mon, 07/30 12:44, Paolo Bonzini wrote: > The "download mbox" file downloads the entire thread, which is rarely > what you want to do. Provide an alternative download link that only > includes the patches and, when the tags module is active, includes the > tags into the mbox. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > mods/tags.py | 47 +++++++++++++++++++++++++++++++- > www/templates/series-detail.html | 6 ++-- > www/urls.py | 1 + > www/views.py | 11 ++++++++ > 4 files changed, 61 insertions(+), 4 deletions(-) > > diff --git a/mods/tags.py b/mods/tags.py > index aa29c60..f7b0300 100644 > --- a/mods/tags.py > +++ b/mods/tags.py > @@ -10,11 +10,13 @@ > > from mod import PatchewModule > from mbox import parse_address > +from django.http import HttpResponse, Http404 > from event import register_handler, emit_event, declare_event > from api.models import Message > from api.rest import PluginMethodField > from patchew.tags import lines_iter > import re > +import www.views > > REV_BY_PREFIX = "Reviewed-by:" > BASED_ON_PREFIX = "Based-on:" > @@ -27,6 +29,21 @@ tags = Tested-by, Reported-by, Acked-by, Suggested-by > > BUILT_IN_TAGS = [REV_BY_PREFIX, BASED_ON_PREFIX] > > +_instance = None > + > +# This is monkey-patched into www.views > +def _view_series_mbox_patches(request, project, message_id): > + global _instance > + s = Message.objects.find_series(message_id, project) > + if not s: > + raise Http404("Series not found") > + if not s.is_complete: > + raise Http404("Series not complete") > + mbox = "\n".join(["From %s %s\n" % (x.get_sender_addr(), x.get_asctime()) + \ > + _instance.get_mbox_with_tags(x) for x in s.get_patches()]) > + return HttpResponse(mbox, content_type="text/plain") > +www.views.view_series_mbox_patches = _view_series_mbox_patches Maybe instead use the dispatch_module_hook() approach? > + > class SeriesTagsModule(PatchewModule): > """ > > @@ -49,6 +66,9 @@ series cover letter, patch mail body and their replies. > default_config = _default_config > > def __init__(self): > + global _instance > + assert _instance == None > + _instance = self > register_handler("MessageAdded", self.on_message_added) > declare_event("TagsUpdate", series="message object that is updated") > > @@ -153,4 +173,29 @@ series cover letter, patch mail body and their replies. > "char": "O", > "row_class": "obsolete" > }) > - > + message.extra_links.append({"html": mark_safe(html), "icon": "exchange" }) > + > + # FIXME: what happens with base64 messages? Decoding then encoding again is the best option I can think of. Or just ignore this corner case, then keep this FIXME here. :) Fam > + def get_mbox_with_tags(self, m): > + def result_iter(): > + regex = self.get_tag_regex() > + old_tags = set() > + lines = lines_iter(m.get_mbox()) > + need_minusminusminus = False > + for line in lines: > + if line.startswith('---'): > + need_minusminusminus = True > + break > + yield line > + if regex.match(line): > + old_tags.add(line) > + > + # If no --- line, tags go at the end as there's no better place > + for tag in m.get_property("tags", []): > + if not tag in old_tags: > + yield tag > + if need_minusminusminus: > + yield line > + yield from lines > + > + return '\n'.join(result_iter()) > diff --git a/www/templates/series-detail.html b/www/templates/series-detail.html > index 52b8f96..1735794 100644 > --- a/www/templates/series-detail.html > +++ b/www/templates/series-detail.html > @@ -26,9 +26,9 @@ > {{ series.age }} ago</span> > <ul id="series-links"> > <li> > - <span class="fa fa-download"></span> <a > - href="/{{ project }}/{{ series.message_id }}/mbox">Download series mbox</a></li> > - </li> > + <span class="fa fa-download"></span>Download {% if series.is_complete %}mbox: <a > + href="/{{ project }}/{{ series.message_id }}/patches.mbox">patches</a>, {% endif %}<a > + href="/{{ project }}/{{ series.message_id }}/mbox">full thread</a></li> > {% for op in series.extra_links %} > <li> > <span class="fa fa-{% if op.icon %}{{ op.icon }}{% else %}question{% endif %}"></span> > diff --git a/www/urls.py b/www/urls.py > index 1659ff2..2087fec 100644 > --- a/www/urls.py > +++ b/www/urls.py > @@ -35,5 +35,6 @@ urlpatterns += [ > url(r"^(?P<project>[^/]*)/(?P<message_id>[^/]*)/$", views.view_series_detail, name="series_detail"), > url(r"^(?P<project>[^/]*)/(?P<thread_id>[^/]*)/(?P<message_id>[^/]*)/$", views.view_series_message, name="series_message"), > url(r"^(?P<project>[^/]*)/(?P<message_id>[^/]*)/mbox$", views.view_series_mbox), > + url(r"^(?P<project>[^/]*)/(?P<message_id>[^/]*)/patches.mbox$", views.view_series_mbox_patches), > url(r"^$", views.view_project_list, name="project_list"), > ] > diff --git a/www/views.py b/www/views.py > index d7c60dd..f2765fe 100644 > --- a/www/views.py > +++ b/www/views.py > @@ -245,6 +245,17 @@ def view_series_mbox(request, project, message_id): > x.get_mbox() for x in r]) > return HttpResponse(mbox, content_type="text/plain") > > +def view_series_mbox_patches(request, project, message_id): > + s = api.models.Message.objects.find_series(message_id, project) > + if not s: > + raise Http404("Series not found") > + if not s.is_complete: > + raise Http404("Series not complete") > + mbox = "\n".join(["From %s %s\n" % (x.get_sender_addr(), x.get_asctime()) + \ > + x.get_mbox(x) for x in s.get_patches()]) > + return HttpResponse(mbox, content_type="text/plain") > + > + > def view_series_detail(request, project, message_id): > s = api.models.Message.objects.find_series(message_id, project) > if not s: > -- > 2.17.1 > > _______________________________________________ > Patchew-devel mailing list > Patchew-devel@redhat.com > https://www.redhat.com/mailman/listinfo/patchew-devel _______________________________________________ Patchew-devel mailing list Patchew-devel@redhat.com https://www.redhat.com/mailman/listinfo/patchew-devel
On 02/08/2018 10:48, Fam Zheng wrote: > On Mon, 07/30 12:44, Paolo Bonzini wrote: >> The "download mbox" file downloads the entire thread, which is rarely >> what you want to do. Provide an alternative download link that only >> includes the patches and, when the tags module is active, includes the >> tags into the mbox. >> >> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> >> --- >> mods/tags.py | 47 +++++++++++++++++++++++++++++++- >> www/templates/series-detail.html | 6 ++-- >> www/urls.py | 1 + >> www/views.py | 11 ++++++++ >> 4 files changed, 61 insertions(+), 4 deletions(-) >> >> diff --git a/mods/tags.py b/mods/tags.py >> index aa29c60..f7b0300 100644 >> --- a/mods/tags.py >> +++ b/mods/tags.py >> @@ -10,11 +10,13 @@ >> >> from mod import PatchewModule >> from mbox import parse_address >> +from django.http import HttpResponse, Http404 >> from event import register_handler, emit_event, declare_event >> from api.models import Message >> from api.rest import PluginMethodField >> from patchew.tags import lines_iter >> import re >> +import www.views >> >> REV_BY_PREFIX = "Reviewed-by:" >> BASED_ON_PREFIX = "Based-on:" >> @@ -27,6 +29,21 @@ tags = Tested-by, Reported-by, Acked-by, Suggested-by >> >> BUILT_IN_TAGS = [REV_BY_PREFIX, BASED_ON_PREFIX] >> >> +_instance = None >> + >> +# This is monkey-patched into www.views >> +def _view_series_mbox_patches(request, project, message_id): >> + global _instance >> + s = Message.objects.find_series(message_id, project) >> + if not s: >> + raise Http404("Series not found") >> + if not s.is_complete: >> + raise Http404("Series not complete") >> + mbox = "\n".join(["From %s %s\n" % (x.get_sender_addr(), x.get_asctime()) + \ >> + _instance.get_mbox_with_tags(x) for x in s.get_patches()]) >> + return HttpResponse(mbox, content_type="text/plain") >> +www.views.view_series_mbox_patches = _view_series_mbox_patches > > Maybe instead use the dispatch_module_hook() approach? Yes, or alternatively make tags first-class citizens that the core knows about, similar to how "version" is part of the model and mods/diff.py only provides the UI. In this case, the only purpose of the tags module is to collect them. Paolo >> + >> class SeriesTagsModule(PatchewModule): >> """ >> >> @@ -49,6 +66,9 @@ series cover letter, patch mail body and their replies. >> default_config = _default_config >> >> def __init__(self): >> + global _instance >> + assert _instance == None >> + _instance = self >> register_handler("MessageAdded", self.on_message_added) >> declare_event("TagsUpdate", series="message object that is updated") >> >> @@ -153,4 +173,29 @@ series cover letter, patch mail body and their replies. >> "char": "O", >> "row_class": "obsolete" >> }) >> - >> + message.extra_links.append({"html": mark_safe(html), "icon": "exchange" }) >> + >> + # FIXME: what happens with base64 messages? > > Decoding then encoding again is the best option I can think of. Or just ignore > this corner case, then keep this FIXME here. :) > > Fam > >> + def get_mbox_with_tags(self, m): >> + def result_iter(): >> + regex = self.get_tag_regex() >> + old_tags = set() >> + lines = lines_iter(m.get_mbox()) >> + need_minusminusminus = False >> + for line in lines: >> + if line.startswith('---'): >> + need_minusminusminus = True >> + break >> + yield line >> + if regex.match(line): >> + old_tags.add(line) >> + >> + # If no --- line, tags go at the end as there's no better place >> + for tag in m.get_property("tags", []): >> + if not tag in old_tags: >> + yield tag >> + if need_minusminusminus: >> + yield line >> + yield from lines >> + >> + return '\n'.join(result_iter()) >> diff --git a/www/templates/series-detail.html b/www/templates/series-detail.html >> index 52b8f96..1735794 100644 >> --- a/www/templates/series-detail.html >> +++ b/www/templates/series-detail.html >> @@ -26,9 +26,9 @@ >> {{ series.age }} ago</span> >> <ul id="series-links"> >> <li> >> - <span class="fa fa-download"></span> <a >> - href="/{{ project }}/{{ series.message_id }}/mbox">Download series mbox</a></li> >> - </li> >> + <span class="fa fa-download"></span>Download {% if series.is_complete %}mbox: <a >> + href="/{{ project }}/{{ series.message_id }}/patches.mbox">patches</a>, {% endif %}<a >> + href="/{{ project }}/{{ series.message_id }}/mbox">full thread</a></li> >> {% for op in series.extra_links %} >> <li> >> <span class="fa fa-{% if op.icon %}{{ op.icon }}{% else %}question{% endif %}"></span> >> diff --git a/www/urls.py b/www/urls.py >> index 1659ff2..2087fec 100644 >> --- a/www/urls.py >> +++ b/www/urls.py >> @@ -35,5 +35,6 @@ urlpatterns += [ >> url(r"^(?P<project>[^/]*)/(?P<message_id>[^/]*)/$", views.view_series_detail, name="series_detail"), >> url(r"^(?P<project>[^/]*)/(?P<thread_id>[^/]*)/(?P<message_id>[^/]*)/$", views.view_series_message, name="series_message"), >> url(r"^(?P<project>[^/]*)/(?P<message_id>[^/]*)/mbox$", views.view_series_mbox), >> + url(r"^(?P<project>[^/]*)/(?P<message_id>[^/]*)/patches.mbox$", views.view_series_mbox_patches), >> url(r"^$", views.view_project_list, name="project_list"), >> ] >> diff --git a/www/views.py b/www/views.py >> index d7c60dd..f2765fe 100644 >> --- a/www/views.py >> +++ b/www/views.py >> @@ -245,6 +245,17 @@ def view_series_mbox(request, project, message_id): >> x.get_mbox() for x in r]) >> return HttpResponse(mbox, content_type="text/plain") >> >> +def view_series_mbox_patches(request, project, message_id): >> + s = api.models.Message.objects.find_series(message_id, project) >> + if not s: >> + raise Http404("Series not found") >> + if not s.is_complete: >> + raise Http404("Series not complete") >> + mbox = "\n".join(["From %s %s\n" % (x.get_sender_addr(), x.get_asctime()) + \ >> + x.get_mbox(x) for x in s.get_patches()]) >> + return HttpResponse(mbox, content_type="text/plain") >> + >> + >> def view_series_detail(request, project, message_id): >> s = api.models.Message.objects.find_series(message_id, project) >> if not s: >> -- >> 2.17.1 >> >> _______________________________________________ >> Patchew-devel mailing list >> Patchew-devel@redhat.com >> https://www.redhat.com/mailman/listinfo/patchew-devel _______________________________________________ Patchew-devel mailing list Patchew-devel@redhat.com https://www.redhat.com/mailman/listinfo/patchew-devel
On Thu, 08/02 11:00, Paolo Bonzini wrote: > On 02/08/2018 10:48, Fam Zheng wrote: > > On Mon, 07/30 12:44, Paolo Bonzini wrote: > >> The "download mbox" file downloads the entire thread, which is rarely > >> what you want to do. Provide an alternative download link that only > >> includes the patches and, when the tags module is active, includes the > >> tags into the mbox. > >> > >> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > >> --- > >> mods/tags.py | 47 +++++++++++++++++++++++++++++++- > >> www/templates/series-detail.html | 6 ++-- > >> www/urls.py | 1 + > >> www/views.py | 11 ++++++++ > >> 4 files changed, 61 insertions(+), 4 deletions(-) > >> > >> diff --git a/mods/tags.py b/mods/tags.py > >> index aa29c60..f7b0300 100644 > >> --- a/mods/tags.py > >> +++ b/mods/tags.py > >> @@ -10,11 +10,13 @@ > >> > >> from mod import PatchewModule > >> from mbox import parse_address > >> +from django.http import HttpResponse, Http404 > >> from event import register_handler, emit_event, declare_event > >> from api.models import Message > >> from api.rest import PluginMethodField > >> from patchew.tags import lines_iter > >> import re > >> +import www.views > >> > >> REV_BY_PREFIX = "Reviewed-by:" > >> BASED_ON_PREFIX = "Based-on:" > >> @@ -27,6 +29,21 @@ tags = Tested-by, Reported-by, Acked-by, Suggested-by > >> > >> BUILT_IN_TAGS = [REV_BY_PREFIX, BASED_ON_PREFIX] > >> > >> +_instance = None > >> + > >> +# This is monkey-patched into www.views > >> +def _view_series_mbox_patches(request, project, message_id): > >> + global _instance > >> + s = Message.objects.find_series(message_id, project) > >> + if not s: > >> + raise Http404("Series not found") > >> + if not s.is_complete: > >> + raise Http404("Series not complete") > >> + mbox = "\n".join(["From %s %s\n" % (x.get_sender_addr(), x.get_asctime()) + \ > >> + _instance.get_mbox_with_tags(x) for x in s.get_patches()]) > >> + return HttpResponse(mbox, content_type="text/plain") > >> +www.views.view_series_mbox_patches = _view_series_mbox_patches > > > > Maybe instead use the dispatch_module_hook() approach? > > Yes, or alternatively make tags first-class citizens that the core knows > about, similar to how "version" is part of the model and mods/diff.py > only provides the UI. In this case, the only purpose of the tags module > is to collect them. No objection from a modeling perspective, but having all tags stored in a JSON field is probably more efficient to a 1:N relationship like Results. Fam _______________________________________________ Patchew-devel mailing list Patchew-devel@redhat.com https://www.redhat.com/mailman/listinfo/patchew-devel
On 02/08/2018 11:13, Fam Zheng wrote: >> Yes, or alternatively make tags first-class citizens that the core knows >> about, similar to how "version" is part of the model and mods/diff.py >> only provides the UI. In this case, the only purpose of the tags module >> is to collect them. > No objection from a modeling perspective, but having all tags stored in a JSON > field is probably more efficient to a 1:N relationship like Results. Yes, I would keep them a JSONField. No reason to overcomplicate the db. Paolo _______________________________________________ Patchew-devel mailing list Patchew-devel@redhat.com https://www.redhat.com/mailman/listinfo/patchew-devel
© 2016 - 2025 Red Hat, Inc.