diff mbox

patch: Add a way to access a patch by its Message-ID

Message ID 1443732965-28022-1-git-send-email-damien.lespiau@intel.com
State Rejected
Headers show

Commit Message

Damien Lespiau Oct. 1, 2015, 8:56 p.m. UTC
Similar to:
http://mid.gmane.org/1443511466-8017-1-git-send-email-jani.nikula@intel.com

It's really handy to be able to find a patchwork patch from the original
mail. For instance, a key stroke in mutt will open the patch page in the
browser.

Suggested-by: Jani Nikula <jani.nikula <at> intel.com>
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 patchwork/urls.py        | 1 +
 patchwork/views/patch.py | 6 +++++-
 2 files changed, 6 insertions(+), 1 deletion(-)

Comments

Stephen Finucane Nov. 4, 2015, 2:53 a.m. UTC | #1
> Similar to:

> http://mid.gmane.org/1443511466-8017-1-git-send-email-jani.nikula@intel.com

> 

> It's really handy to be able to find a patchwork patch from the original

> mail. For instance, a key stroke in mutt will open the patch page in the

> browser.


Nice. Clearly a useful feature: sorry this took so long to get to...

> Suggested-by: Jani Nikula <jani.nikula <at> intel.com>


nit: @

> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>

> ---

>  patchwork/urls.py        | 1 +

>  patchwork/views/patch.py | 6 +++++-

>  2 files changed, 6 insertions(+), 1 deletion(-)

> 

> diff --git a/patchwork/urls.py b/patchwork/urls.py

> index b28eb90..bc792d0 100644

> --- a/patchwork/urls.py

> +++ b/patchwork/urls.py

> @@ -35,6 +35,7 @@ urlpatterns = patterns('',

>      (r'^patch/(?P<patch_id>\d+)/$', 'patchwork.views.patch.patch'),

>      (r'^patch/(?P<patch_id>\d+)/raw/$', 'patchwork.views.patch.content'),

>      (r'^patch/(?P<patch_id>\d+)/mbox/$', 'patchwork.views.patch.mbox'),

> +    (r'^patch/msgid/(?P<msgid>[^/]+)/$', 'patchwork.views.patch.msgid'),


Hmm, is there any reason for using the '/msgid/' prefix? It seems more natural to use the same endpoint for both rather than redirect.

	patch/abc123xyz.123456789@example.com/ 
	patch/1/

This would allow the mbox/patch format downloading to work when using a msgid, which doesn't seem possible right now.

	patch/abc123xyz.123456789@example.com/mbox/
	patch/1/mbox/

You could also do redirects if you think this cleaner, but it should still possible to drop the 'msgid' prefix (Django won't match on the existing cases because of non-digits). This would require three new urlpatterns however: web, raw and mbox.

The current solution works very well for stuff like mutt integration, but I could envision this also being useful for someone who's reading mailing list archives and wants to download the mbox for that from patchwork. I have personally found myself in the position where I haven't subscribed to a mailing and want to access a patch.

>      # logged-in user stuff

>      (r'^user/$', 'patchwork.views.user.profile'),

> diff --git a/patchwork/views/patch.py b/patchwork/views/patch.py

> index 62ff853..f350ec0 100644

> --- a/patchwork/views/patch.py

> +++ b/patchwork/views/patch.py

> @@ -21,7 +21,7 @@

>  from patchwork.models import Patch, Project, Bundle

>  from patchwork.forms import PatchForm, CreateBundleForm

>  from patchwork.requestcontext import PatchworkRequestContext

> -from django.shortcuts import render_to_response, get_object_or_404

> +from django.shortcuts import render_to_response, get_object_or_404,

> redirect

>  from django.http import HttpResponse, HttpResponseForbidden

>  from patchwork.views import generic_list, patch_to_mbox

> 

> @@ -105,3 +105,7 @@ def list(request, project_id):

>      context = generic_list(request, project, 'patchwork.views.patch.list',

>              view_args = {'project_id': project.linkname})

>      return render_to_response('patchwork/list.html', context)

> +

> +def msgid(request, msgid):

> +    patch = get_object_or_404(Patch, msgid='<' + msgid + '>')

> +    return redirect(patch)


Unit tests?

Cheers,
Stephen
Stephen Finucane Nov. 12, 2015, 4:45 a.m. UTC | #2
On 04 Nov 02:53, Finucane, Stephen wrote:
> > Similar to:
> > http://mid.gmane.org/1443511466-8017-1-git-send-email-jani.nikula@intel.com
> > 
> > It's really handy to be able to find a patchwork patch from the original
> > mail. For instance, a key stroke in mutt will open the patch page in the
> > browser.
> 
> Nice. Clearly a useful feature: sorry this took so long to get to...
> 
> > Suggested-by: Jani Nikula <jani.nikula <at> intel.com>
> 
> nit: @
> 
> > Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> > ---
> >  patchwork/urls.py        | 1 +
> >  patchwork/views/patch.py | 6 +++++-
> >  2 files changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/patchwork/urls.py b/patchwork/urls.py
> > index b28eb90..bc792d0 100644
> > --- a/patchwork/urls.py
> > +++ b/patchwork/urls.py
> > @@ -35,6 +35,7 @@ urlpatterns = patterns('',
> >      (r'^patch/(?P<patch_id>\d+)/$', 'patchwork.views.patch.patch'),
> >      (r'^patch/(?P<patch_id>\d+)/raw/$', 'patchwork.views.patch.content'),
> >      (r'^patch/(?P<patch_id>\d+)/mbox/$', 'patchwork.views.patch.mbox'),
> > +    (r'^patch/msgid/(?P<msgid>[^/]+)/$', 'patchwork.views.patch.msgid'),
> 
> Hmm, is there any reason for using the '/msgid/' prefix? It seems more natural to use the same endpoint for both rather than redirect.
> 
> 	patch/abc123xyz.123456789@example.com/ 
> 	patch/1/
> 
> This would allow the mbox/patch format downloading to work when using a msgid, which doesn't seem possible right now.
> 
> 	patch/abc123xyz.123456789@example.com/mbox/
> 	patch/1/mbox/
> 
> You could also do redirects if you think this cleaner, but it should still possible to drop the 'msgid' prefix (Django won't match on the existing cases because of non-digits). This would require three new urlpatterns however: web, raw and mbox.
> 
> The current solution works very well for stuff like mutt integration, but I could envision this also being useful for someone who's reading mailing list archives and wants to download the mbox for that from patchwork. I have personally found myself in the position where I haven't subscribed to a mailing and want to access a patch.
> 
> >      # logged-in user stuff
> >      (r'^user/$', 'patchwork.views.user.profile'),
> > diff --git a/patchwork/views/patch.py b/patchwork/views/patch.py
> > index 62ff853..f350ec0 100644
> > --- a/patchwork/views/patch.py
> > +++ b/patchwork/views/patch.py
> > @@ -21,7 +21,7 @@
> >  from patchwork.models import Patch, Project, Bundle
> >  from patchwork.forms import PatchForm, CreateBundleForm
> >  from patchwork.requestcontext import PatchworkRequestContext
> > -from django.shortcuts import render_to_response, get_object_or_404
> > +from django.shortcuts import render_to_response, get_object_or_404,
> > redirect
> >  from django.http import HttpResponse, HttpResponseForbidden
> >  from patchwork.views import generic_list, patch_to_mbox
> > 
> > @@ -105,3 +105,7 @@ def list(request, project_id):
> >      context = generic_list(request, project, 'patchwork.views.patch.list',
> >              view_args = {'project_id': project.linkname})
> >      return render_to_response('patchwork/list.html', context)
> > +
> > +def msgid(request, msgid):
> > +    patch = get_object_or_404(Patch, msgid='<' + msgid + '>')
> > +    return redirect(patch)
> 
> Unit tests?
> 
> Cheers,
> Stephen

Bump.
diff mbox

Patch

diff --git a/patchwork/urls.py b/patchwork/urls.py
index b28eb90..bc792d0 100644
--- a/patchwork/urls.py
+++ b/patchwork/urls.py
@@ -35,6 +35,7 @@  urlpatterns = patterns('',
     (r'^patch/(?P<patch_id>\d+)/$', 'patchwork.views.patch.patch'),
     (r'^patch/(?P<patch_id>\d+)/raw/$', 'patchwork.views.patch.content'),
     (r'^patch/(?P<patch_id>\d+)/mbox/$', 'patchwork.views.patch.mbox'),
+    (r'^patch/msgid/(?P<msgid>[^/]+)/$', 'patchwork.views.patch.msgid'),
 
     # logged-in user stuff
     (r'^user/$', 'patchwork.views.user.profile'),
diff --git a/patchwork/views/patch.py b/patchwork/views/patch.py
index 62ff853..f350ec0 100644
--- a/patchwork/views/patch.py
+++ b/patchwork/views/patch.py
@@ -21,7 +21,7 @@ 
 from patchwork.models import Patch, Project, Bundle
 from patchwork.forms import PatchForm, CreateBundleForm
 from patchwork.requestcontext import PatchworkRequestContext
-from django.shortcuts import render_to_response, get_object_or_404
+from django.shortcuts import render_to_response, get_object_or_404, redirect
 from django.http import HttpResponse, HttpResponseForbidden
 from patchwork.views import generic_list, patch_to_mbox
 
@@ -105,3 +105,7 @@  def list(request, project_id):
     context = generic_list(request, project, 'patchwork.views.patch.list',
             view_args = {'project_id': project.linkname})
     return render_to_response('patchwork/list.html', context)
+
+def msgid(request, msgid):
+    patch = get_object_or_404(Patch, msgid='<' + msgid + '>')
+    return redirect(patch)