Bug#762944: debsources: make .pc/ exclusion a configuration parameter
Hello again,
As discussed on IRC, I have rebased my patches for this bug.
Cheers
--
Jason Pleau
From ee67677ce389a844221e3a712598147faf1ff2ff Mon Sep 17 00:00:00 2001
From: Jason Pleau ja...@jpleau.ca
Date: Mon, 9 Mar 2015 12:34:06 -0400
Subject: [PATCH 1/2] add a new hidden_files setting in config.ini
Right now the .pc/ exclusion in directory listings is hardcoded. This
new hidden_files configuration allows us to be more flexible in what is
shown or hidden in those listings.
*/.pc/ is added as a default value for this configuration setting.
Closes #762944
---
.../sources/templates/sources/source_folder.html | 48 --
debsources/app/sources/views.py| 6 +--
debsources/mainlib.py | 4 +-
debsources/models.py | 18 +---
debsources/tests/test_webapp.py| 22 ++
doc/examples/sample-config.local.ini | 12 ++
etc/config.ini | 12 ++
7 files changed, 89 insertions(+), 33 deletions(-)
diff --git a/debsources/app/sources/templates/sources/source_folder.html b/debsources/app/sources/templates/sources/source_folder.html
index ba6f894..ac3b5b4 100644
--- a/debsources/app/sources/templates/sources/source_folder.html
+++ b/debsources/app/sources/templates/sources/source_folder.html
@@ -32,31 +32,35 @@
/tr
{% for dir in subdirs %}
- tr
-td class=item-imgimg src={{ config['ICONS_FOLDER'] }}22x22/places/folder.png alt=d //td
-{% if config[DIR_LS_LONG] %}td class=stat-typespan{{ dir.stat.type }}/span/td{% endif %}
-td class=stat-permsspan{{ dir.stat.perms }}/span/td
-{% if config[DIR_LS_LONG] %}td class=stat-sizespan{{ {:,d}.format(dir.stat.size) }}/span/td{% endif %}
-td class=item-namea href={{ url_for('.source', path_to=path+'/'+dir.name) }}{{ dir.name }}/a
- {% if config[DIR_LS_LONG] %}
-{% if dir.stat.symlink_dest is not none %}{{ â + dir.stat.symlink_dest }}{% endif %}
- {% endif %}
-/td
- /tr
+ {% if not dir['hidden'] %}
+tr
+ td class=item-imgimg src={{ config['ICONS_FOLDER'] }}22x22/places/folder.png alt=d //td
+ {% if config[DIR_LS_LONG] %}td class=stat-typespan{{ dir.stat.type }}/span/td{% endif %}
+ td class=stat-permsspan{{ dir.stat.perms }}/span/td
+ {% if config[DIR_LS_LONG] %}td class=stat-sizespan{{ {:,d}.format(dir.stat.size) }}/span/td{% endif %}
+ td class=item-namea href={{ url_for('.source', path_to=path+'/'+dir.name) }}{{ dir.name }}/a
+{% if config[DIR_LS_LONG] %}
+ {% if dir.stat.symlink_dest is not none %}{{ â + dir.stat.symlink_dest }}{% endif %}
+{% endif %}
+ /td
+/tr
+ {% endif %}
{% endfor %}
{% for file_ in subfiles %}
- tr
-td class=item-imgimg src={{ config['ICONS_FOLDER'] }}22x22/mimetypes/ascii.png alt=- //td
-{% if config[DIR_LS_LONG] %}td class=stat-typespan{{ file_.stat.type }}/span/td{% endif %}
-td class=stat-permsspan{{ file_.stat.perms }}/span/td
-{% if config[DIR_LS_LONG] %}td class=stat-sizespan{{ {:,d}.format(file_.stat.size) }}/span/td{% endif %}
-td class=item-namea href={{ url_for('.source', path_to=path+'/'+file_.name) }}{{ file_.name }}/a
- {% if config[DIR_LS_LONG] %}
-{% if file_.stat.symlink_dest is not none %}{{ â + file_.stat.symlink_dest }}{% endif %}
- {% endif %}
-/td
- /tr
+ {% if not file_['hidden'] %}
+tr
+ td class=item-imgimg src={{ config['ICONS_FOLDER'] }}22x22/mimetypes/ascii.png alt=- //td
+ {% if config[DIR_LS_LONG] %}td class=stat-typespan{{ file_.stat.type }}/span/td{% endif %}
+ td class=stat-permsspan{{ file_.stat.perms }}/span/td
+ {% if config[DIR_LS_LONG] %}td class=stat-sizespan{{ {:,d}.format(file_.stat.size) }}/span/td{% endif %}
+ td class=item-namea href={{ url_for('.source', path_to=path+'/'+file_.name) }}{{ file_.name }}/a
+{% if config[DIR_LS_LONG] %}
+ {% if file_.stat.symlink_dest is not none %}{{ â + file_.stat.symlink_dest }}{% endif %}
+{% endif %}
+ /td
+/tr
+ {% endif %}
{% endfor %}
/table
diff --git a/debsources/app/sources/views.py b/debsources/app/sources/views.py
index e6c6bf1..72c30b4 100644
--- a/debsources/app/sources/views.py
+++ b/debsources/app/sources/views.py
@@ -137,10 +137,8 @@ class SourceView(GeneralView):
renders a directory, lists subdirs and subfiles
-directory = Directory(location, toplevel=(location.get_path() == ))
-
-# (if path == , then the dir is toplevel, and we don't want
-# the .pc directory)
+hidden_files = app.config['HIDDEN_FILES'].split( )
+directory = Directory(location, hidden_files)
pkg_infos = Infobox(session, location.get_package(),
location.get_version()).get_infos()
diff --git a/debsources/mainlib.py b/debsources/mainlib.py
index
Bug#762944: debsources: make .pc/ exclusion a configuration parameter
On Mon, Mar 09, 2015 at 02:59:54PM -0400, Jason Pleau wrote: Updated patch that fixes an error if hidden_files is empty Hey Jason, thanks a lot for your patch! It looks great in general. I just have a few minor changes to request, if you don't mind. See below for details. Right now the .pc/ exclusion in directory listings is hardcoded. This new hidden_files configuration allows us to be more flexible in what is shown or hidden in those listings. */.pc/ is added as a default value for this configuration setting. This is one concern I have. You've modified the sample config file, but not the default configuration stored in mainlib.py. I haven't tested that, but I suspect that for that reason upgrading a currently deployed Debsources instance to a version that include your patch will trigger failures at runtime, if config.local.ini isn't updated. Can you confirm that? If so, I'd very much prefer changing DEFAULT_CONFIG in mainlib.py to include */.pc/ as default setting to 1) avoid breakages, and 2) retain the current behavior. diff --git a/debsources/app/templates/source_folder.html b/debsources/app/templates/source_folder.html Template and view changes look good, and I daresay that models look nicer with your changes :) +# space-separated list of files or directories patterns to hide in +# directory listings +hidden_files: */.pc/ It would be nice to document in the comment here (for lack of a better place...) the intended semantics of hidden_files, i.e.: match on the full path, relative to which dir, and maybe the fact that fnmatch() is involved, for reference. There are also a couple of minor flake8 issues in your patch (which I can fix upon patch import, if needed). And from your other email: This also allow us to add a toggle button (show / hide hidden files) in directory listings eventually. I could do that in a separate commit in this bug if you'd like. Yes please, that would be awesome! :) Cheers. -- Stefano Zacchiroli . . . . . . . z...@upsilon.cc . . . . o . . . o . o Maître de conférences . . . . . http://upsilon.cc/zack . . . o . . . o o Former Debian Project Leader . . @zack on identi.ca . . o o o . . . o . « the first rule of tautology club is the first rule of tautology club » -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#762944: debsources: make .pc/ exclusion a configuration parameter
Hello Stefano,
I have attached two patches. One fixes the issues we talked about
earlier today, and the other implements the Show / Hide hidden files
button in directory listing:
On 10/03/15 09:43 AM, Stefano Zacchiroli wrote:
On Mon, Mar 09, 2015 at 02:59:54PM -0400, Jason Pleau wrote:
Updated patch that fixes an error if hidden_files is empty
Hey Jason, thanks a lot for your patch! It looks great in general. I
just have a few minor changes to request, if you don't mind. See below
for details.
Right now the .pc/ exclusion in directory listings is hardcoded. This
new hidden_files configuration allows us to be more flexible in what is
shown or hidden in those listings.
*/.pc/ is added as a default value for this configuration setting.
This is one concern I have. You've modified the sample config file, but
not the default configuration stored in mainlib.py. I haven't tested
that, but I suspect that for that reason upgrading a currently deployed
Debsources instance to a version that include your patch will trigger
failures at runtime, if config.local.ini isn't updated. Can you confirm
that?
If so, I'd very much prefer changing DEFAULT_CONFIG in mainlib.py to
include */.pc/ as default setting to 1) avoid breakages, and 2) retain
the current behavior.
As talked on IRC, I have added */.pc/ to DEFAULT_CONFIG.web_app.
+# space-separated list of files or directories patterns to hide in
+# directory listings
+hidden_files: */.pc/
It would be nice to document in the comment here (for lack of a better
place...) the intended semantics of hidden_files, i.e.: match on the
full path, relative to which dir, and maybe the fact that fnmatch() is
involved, for reference.
I have added a bit more comments, feel free to adjust if you think you
can phrase it better than it is currently (I have no shame in admitting
that I may sometimes not be clear enough in English :) )
There are also a couple of minor flake8 issues in your patch (which I
can fix upon patch import, if needed).
They now seem fixed on my side, you can re-run flake8 to confirm !
And from your other email:
This also allow us to add a toggle button (show / hide hidden files) in
directory listings eventually. I could do that in a separate commit in
this bug if you'd like.
Yes please, that would be awesome! :)
Done :) See 0002 patch !
Cheers.
Thanks
--
Jason Pleau
From 641a210eb0d06dfc63a9332891981bcfd07e3426 Mon Sep 17 00:00:00 2001
From: Jason Pleau ja...@jpleau.ca
Date: Tue, 10 Mar 2015 20:37:22 -0400
Subject: [PATCH 2/2] source_folder: add a show/hide hidden files button
Since that hidden files and directories are still passed to the view,
but are now given an 'hidden' boolean attribute, we can show or hide
them once the page is loaded.
I also changed the way javascript was loaded in debsources.js, so that
it does not run unnecessary code (for example we don't need to print
line numbers in folders).
---
debsources/app/static/css/source_folder.css| 13 ++
debsources/app/static/javascript/debsources.js | 273 ++---
debsources/app/templates/source_file.html | 2 +-
debsources/app/templates/source_folder.html| 59 +++---
debsources/app/views.py| 1 +
5 files changed, 196 insertions(+), 152 deletions(-)
diff --git a/debsources/app/static/css/source_folder.css b/debsources/app/static/css/source_folder.css
index 1a45b8f..1bd9c6e 100644
--- a/debsources/app/static/css/source_folder.css
+++ b/debsources/app/static/css/source_folder.css
@@ -40,3 +40,16 @@ License: GNU Affero General Public License, version 3 or above.
font-size: small;
text-align: right;
}
+
+.hidden_file {
+display: none;
+}
+
+.hidden_file.visible {
+display: table-row;
+}
+
+#btn_toggle_hidden_files {
+ font-size: 0.8em;
+}
+
diff --git a/debsources/app/static/javascript/debsources.js b/debsources/app/static/javascript/debsources.js
index 844e58a..ced5cf6 100644
--- a/debsources/app/static/javascript/debsources.js
+++ b/debsources/app/static/javascript/debsources.js
@@ -1,4 +1,4 @@
-/* Copyright (C) 2014 Jason Pleau ja...@jpleau.ca
+/* Copyright (C) 2015 Jason Pleau ja...@jpleau.ca
*
* This file is part of Debsources.
*
@@ -27,128 +27,151 @@
*
*/
-var debsources = function(message_pos) {
-var print_lines = function() {
-var position = message_pos;
-var msgbox = document.getElementById('messages');
-var index = document.getElementById('sourceslinenumbers');
-var divHeight = msgbox.offsetHeight;
-var lineHeight = parseInt(window.getComputedStyle(index).getPropertyValue('line-height'),10);
-var lines = Math.ceil(divHeight / lineHeight)+1; // always insert one more line below the last line of code
-
-for(i=0; ilines; ++i){
-var element = document.createElement('a');
-var s = 'a/abr'; // lines corr. messages do no need indexes
-element.innerHTML
Bug#762944: debsources: make .pc/ exclusion a configuration parameter
Hi Stefano.
I have a bit on this bug, please see the attached patch.
Instead of filtering out files that should be hidden, I have added an
'hidden' (bool) key to the dictionary that it returns, so that the API
can return all the files, even if they're hidden. It should not be the
model nor the controller that decides what is shown on the screen, but
the view.
This also allow us to add a toggle button (show / hide hidden files) in
directory listings eventually. I could do that in a separate commit in
this bug if you'd like.
Cheers !
--
Jason Pleau
From 6b2b89b6e63baa089f4f671286a69cc2b22a915f Mon Sep 17 00:00:00 2001
From: Jason Pleau ja...@jpleau.ca
Date: Mon, 9 Mar 2015 12:34:06 -0400
Subject: [PATCH] add a new hidden_files setting in config.ini
Right now the .pc/ exclusion in directory listings is hardcoded. This
new hidden_files configuration allows us to be more flexible in what is
shown or hidden in those listings.
*/.pc/ is added as a default value for this configuration setting.
Closes #762944
---
debsources/app/templates/source_folder.html | 48 -
debsources/app/views.py | 5 ++-
debsources/models.py| 18 +++
debsources/tests/test_webapp.py | 21 +
doc/examples/sample-config.local.ini| 4 +++
etc/config.ini | 4 +++
6 files changed, 69 insertions(+), 31 deletions(-)
diff --git a/debsources/app/templates/source_folder.html b/debsources/app/templates/source_folder.html
index 444194a..99ca1c1 100644
--- a/debsources/app/templates/source_folder.html
+++ b/debsources/app/templates/source_folder.html
@@ -32,31 +32,35 @@
/tr
{% for dir in subdirs %}
- tr
-td class=item-imgimg src={{ config['ICONS_FOLDER'] }}22x22/places/folder.png alt=d //td
-{% if config[DIR_LS_LONG] %}td class=stat-typespan{{ dir.stat.type }}/span/td{% endif %}
-td class=stat-permsspan{{ dir.stat.perms }}/span/td
-{% if config[DIR_LS_LONG] %}td class=stat-sizespan{{ {:,d}.format(dir.stat.size) }}/span/td{% endif %}
-td class=item-namea href={{ url_for('source_html', path_to=path+'/'+dir.name) }}{{ dir.name }}/a
- {% if config[DIR_LS_LONG] %}
-{% if dir.stat.symlink_dest is not none %}{{ â + dir.stat.symlink_dest }}{% endif %}
- {% endif %}
-/td
- /tr
+ {% if not dir['hidden'] %}
+tr
+ td class=item-imgimg src={{ config['ICONS_FOLDER'] }}22x22/places/folder.png alt=d //td
+ {% if config[DIR_LS_LONG] %}td class=stat-typespan{{ dir.stat.type }}/span/td{% endif %}
+ td class=stat-permsspan{{ dir.stat.perms }}/span/td
+ {% if config[DIR_LS_LONG] %}td class=stat-sizespan{{ {:,d}.format(dir.stat.size) }}/span/td{% endif %}
+ td class=item-namea href={{ url_for('source_html', path_to=path+'/'+dir.name) }}{{ dir.name }}/a
+{% if config[DIR_LS_LONG] %}
+ {% if dir.stat.symlink_dest is not none %}{{ â + dir.stat.symlink_dest }}{% endif %}
+{% endif %}
+ /td
+/tr
+ {% endif %}
{% endfor %}
{% for file_ in subfiles %}
- tr
-td class=item-imgimg src={{ config['ICONS_FOLDER'] }}22x22/mimetypes/ascii.png alt=- //td
-{% if config[DIR_LS_LONG] %}td class=stat-typespan{{ file_.stat.type }}/span/td{% endif %}
-td class=stat-permsspan{{ file_.stat.perms }}/span/td
-{% if config[DIR_LS_LONG] %}td class=stat-sizespan{{ {:,d}.format(file_.stat.size) }}/span/td{% endif %}
-td class=item-namea href={{ url_for('source_html', path_to=path+'/'+file_.name) }}{{ file_.name }}/a
- {% if config[DIR_LS_LONG] %}
-{% if file_.stat.symlink_dest is not none %}{{ â + file_.stat.symlink_dest }}{% endif %}
- {% endif %}
-/td
- /tr
+ {% if not file_['hidden'] %}
+tr
+ td class=item-imgimg src={{ config['ICONS_FOLDER'] }}22x22/mimetypes/ascii.png alt=- //td
+ {% if config[DIR_LS_LONG] %}td class=stat-typespan{{ file_.stat.type }}/span/td{% endif %}
+ td class=stat-permsspan{{ file_.stat.perms }}/span/td
+ {% if config[DIR_LS_LONG] %}td class=stat-sizespan{{ {:,d}.format(file_.stat.size) }}/span/td{% endif %}
+ td class=item-namea href={{ url_for('source_html', path_to=path+'/'+file_.name) }}{{ file_.name }}/a
+{% if config[DIR_LS_LONG] %}
+ {% if file_.stat.symlink_dest is not none %}{{ â + file_.stat.symlink_dest }}{% endif %}
+{% endif %}
+ /td
+/tr
+ {% endif %}
{% endfor %}
/table
diff --git a/debsources/app/views.py b/debsources/app/views.py
index 2519c5f..6a99377 100644
--- a/debsources/app/views.py
+++ b/debsources/app/views.py
@@ -554,10 +554,9 @@ class SourceView(GeneralView):
renders a directory, lists subdirs and subfiles
-directory = Directory(location, toplevel=(location.get_path() == ))
-# (if path == , then the dir is toplevel, and we don't want
-# the .pc directory)
+hidden_files =
Bug#762944: debsources: make .pc/ exclusion a configuration parameter
Updated patch that fixes an error if hidden_files is empty
Sorry for the noise :)
From 275b94fe8031ac81c75388190dcd58709b14f8d7 Mon Sep 17 00:00:00 2001
From: Jason Pleau ja...@jpleau.ca
Date: Mon, 9 Mar 2015 12:34:06 -0400
Subject: [PATCH] add a new hidden_files setting in config.ini
Right now the .pc/ exclusion in directory listings is hardcoded. This
new hidden_files configuration allows us to be more flexible in what is
shown or hidden in those listings.
*/.pc/ is added as a default value for this configuration setting.
Closes #762944
---
debsources/app/templates/source_folder.html | 48 -
debsources/app/views.py | 5 ++-
debsources/models.py| 18 +++
debsources/tests/test_webapp.py | 21 +
doc/examples/sample-config.local.ini| 4 +++
etc/config.ini | 4 +++
6 files changed, 69 insertions(+), 31 deletions(-)
diff --git a/debsources/app/templates/source_folder.html b/debsources/app/templates/source_folder.html
index 444194a..99ca1c1 100644
--- a/debsources/app/templates/source_folder.html
+++ b/debsources/app/templates/source_folder.html
@@ -32,31 +32,35 @@
/tr
{% for dir in subdirs %}
- tr
-td class=item-imgimg src={{ config['ICONS_FOLDER'] }}22x22/places/folder.png alt=d //td
-{% if config[DIR_LS_LONG] %}td class=stat-typespan{{ dir.stat.type }}/span/td{% endif %}
-td class=stat-permsspan{{ dir.stat.perms }}/span/td
-{% if config[DIR_LS_LONG] %}td class=stat-sizespan{{ {:,d}.format(dir.stat.size) }}/span/td{% endif %}
-td class=item-namea href={{ url_for('source_html', path_to=path+'/'+dir.name) }}{{ dir.name }}/a
- {% if config[DIR_LS_LONG] %}
-{% if dir.stat.symlink_dest is not none %}{{ â + dir.stat.symlink_dest }}{% endif %}
- {% endif %}
-/td
- /tr
+ {% if not dir['hidden'] %}
+tr
+ td class=item-imgimg src={{ config['ICONS_FOLDER'] }}22x22/places/folder.png alt=d //td
+ {% if config[DIR_LS_LONG] %}td class=stat-typespan{{ dir.stat.type }}/span/td{% endif %}
+ td class=stat-permsspan{{ dir.stat.perms }}/span/td
+ {% if config[DIR_LS_LONG] %}td class=stat-sizespan{{ {:,d}.format(dir.stat.size) }}/span/td{% endif %}
+ td class=item-namea href={{ url_for('source_html', path_to=path+'/'+dir.name) }}{{ dir.name }}/a
+{% if config[DIR_LS_LONG] %}
+ {% if dir.stat.symlink_dest is not none %}{{ â + dir.stat.symlink_dest }}{% endif %}
+{% endif %}
+ /td
+/tr
+ {% endif %}
{% endfor %}
{% for file_ in subfiles %}
- tr
-td class=item-imgimg src={{ config['ICONS_FOLDER'] }}22x22/mimetypes/ascii.png alt=- //td
-{% if config[DIR_LS_LONG] %}td class=stat-typespan{{ file_.stat.type }}/span/td{% endif %}
-td class=stat-permsspan{{ file_.stat.perms }}/span/td
-{% if config[DIR_LS_LONG] %}td class=stat-sizespan{{ {:,d}.format(file_.stat.size) }}/span/td{% endif %}
-td class=item-namea href={{ url_for('source_html', path_to=path+'/'+file_.name) }}{{ file_.name }}/a
- {% if config[DIR_LS_LONG] %}
-{% if file_.stat.symlink_dest is not none %}{{ â + file_.stat.symlink_dest }}{% endif %}
- {% endif %}
-/td
- /tr
+ {% if not file_['hidden'] %}
+tr
+ td class=item-imgimg src={{ config['ICONS_FOLDER'] }}22x22/mimetypes/ascii.png alt=- //td
+ {% if config[DIR_LS_LONG] %}td class=stat-typespan{{ file_.stat.type }}/span/td{% endif %}
+ td class=stat-permsspan{{ file_.stat.perms }}/span/td
+ {% if config[DIR_LS_LONG] %}td class=stat-sizespan{{ {:,d}.format(file_.stat.size) }}/span/td{% endif %}
+ td class=item-namea href={{ url_for('source_html', path_to=path+'/'+file_.name) }}{{ file_.name }}/a
+{% if config[DIR_LS_LONG] %}
+ {% if file_.stat.symlink_dest is not none %}{{ â + file_.stat.symlink_dest }}{% endif %}
+{% endif %}
+ /td
+/tr
+ {% endif %}
{% endfor %}
/table
diff --git a/debsources/app/views.py b/debsources/app/views.py
index 2519c5f..6a99377 100644
--- a/debsources/app/views.py
+++ b/debsources/app/views.py
@@ -554,10 +554,9 @@ class SourceView(GeneralView):
renders a directory, lists subdirs and subfiles
-directory = Directory(location, toplevel=(location.get_path() == ))
-# (if path == , then the dir is toplevel, and we don't want
-# the .pc directory)
+hidden_files = app.config['HIDDEN_FILES'].split( )
+directory = Directory(location, hidden_files)
pkg_infos = Infobox(session,
location.get_package(),
diff --git a/debsources/models.py b/debsources/models.py
index e4d05fc..de015db 100644
--- a/debsources/models.py
+++ b/debsources/models.py
@@ -19,6 +19,7 @@
import os
import magic
import stat
+import fnmatch
from collections import namedtuple
from sqlalchemy import Column, ForeignKey
@@ -652,11
Bug#762944: debsources: make .pc/ exclusion a configuration parameter
Package: qa.debian.org Severity: wishlist User: qa.debian@packages.debian.org Usertags: debsources Currently, the exclusion of the .pc/ dir from source package navigation is hard-coded, see: http://anonscm.debian.org/cgit/qa/debsources.git/tree/debsources/models.py#n578 It should become a configuration entry, likely using file-globbing. -- System Information: Debian Release: jessie/sid APT prefers testing APT policy: (500, 'testing'), (1, 'experimental') Architecture: amd64 (x86_64) Foreign Architectures: i386 Kernel: Linux 3.14-2-amd64 (SMP w/4 CPU cores) Locale: LANG=it_IT.UTF-8, LC_CTYPE=it_IT.UTF-8 (charmap=UTF-8) Shell: /bin/sh linked to /bin/dash -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
