Issue #687 Added ability to specify query and fragment elements of url via linkify.

[benmehlman]

Squashed commit of the following:

commit f0199f2
Author: Ben Mehlman ben.z.mehlman@gmail.com
Date: Wed Nov 6 15:07:42 2019 -0500

When passing linkify as dict, QUERY and FRAGMENT now lowercase (ie. query, fragment).
Update documentation to explain these features.

commit e14b529
Author: Ben Mehlman ben.z.mehlman@gmail.com
Date: Tue Nov 5 22:22:01 2019 -0500

Add query string and fragment identifier capability to linkify.

[benmehlman]

Sorry, it looks like I've messed up the documentation somehow.. but Travis doesn't tell me the actual spelling error... If you could tell me what I've messed up I'll fix it, thanks!

[jieter]

@benmehlman docs, flake8 and black are failing currently, I suspect running black . will cure the latter two.

As for the docs, the errors are a bit buried in deprecation warnings and progress reporting, but the relevant error seems to be:

pages/api-reference.rst:65:url:["URL", "curl", "purl", "hurl", "burl", "furl", "Burl", "urn", "Erl", "yml", "arg"]

using uppercase URL will fix it, or add url to docs/spelling_wordlist.txt

[jieter]

The patch looks reasonable, thanks!

We do however need to have tests for this before I can merge it.

[jieter]

List is also supported per your change in line 326, so can you add it to the docs here too?

[benmehlman]

Thanks. Will do, but I guess this is all going to take some time.. because we are on Debian Jessie here w/ Python 3.4. So I can run flake8, but can't run black (required python3.6) and tables2 tests fail due to too-old version of django_filters. We are in the process of a upgrading but it will take some time. Probably I'll just have to create a debian stretch VM for tables2 development.

[jieter]

you can just push however many times you like to let the CI do it, the logs can be inspected here:

https://travis-ci.org/jieter/django-tables2/pull_requests?utm_medium=notification&utm_source=github_status

[benmehlman]

Thanks, but if I have to write tests I need to be able to run them locally.. so I set up the environment. I've got several other tables2 features I want to offer to the project that I am using here, some implemented using monkeypatches that I have had in place for years, and I just recently found a problem with pagination and order_by that I want to fix (in the meantime, I coded around it by eliminating my use of RequestConfig and overriding SingleTableMixin.get_table()), so I might as well set things up correctly.

So now I've been able to run black and see what it did not like about my formatting, so that's fine now.

But I've got two questions:

1. I've noticed that in the docstrings, you sometimes use single backticks around symbols or code (black monospace) and sometimes double backticks (red monospace). And I looked at the existing docs to see whether there was a rule to this but could not find a consistent rule. Trying to get this right so would appreciate your guidance here.

2. When running tox, in your requirements file I see you specified Django==2.2.3, but it installs Django 3.1.. and this is a problem because:

jazzband/django-oauth-toolkit#752

So the tables2 tests all fail.

So there's legitimate breakage between tables2 HEAD with Django 3.1 and I can fix that, but that should probably be a separate pr... meanwhile I am stuck on the tests, so any help understanding why tox doesn't use the right django is appreciated.

This is my first time using tox so maybe I'm just running it wrong... I more or less followed what you did here: https://gist.github.com/jieter/e0c2f577fe3c685edd04

Thanks!

[jieter]

For 1: I had to look this up, so it is probably not consistent:

The default role (content) has no special meaning by default. You are free to use it for anything you like, e.g. variable names; use the default_role config value to set it to a known role – the any role to find anything or the py:obj role to find Python objects are very useful for this.

http://www.sphinx-doc.org/en/master/usage/restructuredtext/roles.html

If we are going to adopt a rule, it would be some work to bring the existing docstrings into alignment with it, so I'd suggest to not worry about it too much for this PR.

For 2: Tox installs different versions of Django for different entries in the test matrix:

django-tables2/tox.ini

Lines 33 to 36 in 4941e4f

	2.1: Django==2.1.*
	2.2: Django==2.2.*
	3.0: Django==3.0a1
	master: https://github.com/django/django/archive/master.tar.gz

In addition to Django, it also installs a couple of other dependencies: from requirements/common.pip

[benmehlman]

Ok.. added tests for the new code to test_linkcolumn (in addition, added pass-through of new parameters from LinkColumn to linkify and updated docs for LinkColumn)... All tests pass.

[jieter]

I think this does not work for QueryDicts, which is needed to support multiple values for a single key (i.e. id=1&id=2). I'm not sure if thats worth it to add support for.

[benmehlman]

Hi, I am back! Are you still interested in this patch?
If the only think holding it back is support for QueryDict I can make that change.

Or.. I should ask.. is it actual QueryDict support you would like (direct usage of QueryDict class? Or just equivalent functionality, eg. a list would be interpreted as multiple values:

myquery = {
     "a": 1,
     "b": [2, 3, 4, 5]
}

Result: ?a1&b=2&b=3&b=4&b=5

[jieter]

Welcome back ;)

Yes, when looking at it now, multiple values is a must-have.