Modify

Opened 3 years ago

Closed 9 months ago

#9057 closed defect (fixed)

[patch] Tags with single quote not tokenised correctly

Reported by: anonymous Owned by: hasienda
Priority: normal Component: TagsPlugin
Severity: normal Keywords: HTML links tag quote
Cc: rjollos, otaku42 Trac Release: 0.11

Description

I added the tag "single'quote" to a wiki page.

The tag is shown in the tag-cloud, but the results for [[ListTagged(single'quote)]] and for /tags?q='single%5C'quote' (which is how the URL is rendered by default - not sure why) are empty.

I debugged this into tagsplugin/trunk/tractags/query.py Query:_tokenise() that seems to split the phrase on a single quote.

Attachments (1)

tagsplugin-r10682.diff (3.1 KB) - added by jun66j5 3 years ago.

Download all attachments as: .zip

Change History (14)

comment:1 Changed 3 years ago by hasienda

  • Keywords HTML links tag quote added

Confirmed: While the tag-cloud shows correctly formatted tag and object count (1), following the link (here: http://localhost:8000/sandbox/tags/%27single%5C%27quote%27) returns a page with title

Showing objects matching ''single\'quote''

but no matching objects.

Note: tag in tags: name space is rendered differently here (with Trac 0.13dev)

Try with "single'quote"

  • in tag search box and hit <Filter Tags>, this seems to work: returned page (http://localhost:8000/sandbox/tags?q=%22single%27quote%22) has the correct object listed, but the back-link to the tag is still wrong (http://localhost:8000/sandbox/tags/%27single%5C%27quote%27)
  • [[ListTagged("single'quote")]] works here as well.

Note: single double-quotes instead of pair of single quotes like above

comment:2 follow-up: Changed 3 years ago by hasienda

While resolving other issues I've been stumbling over the rather strange quoting that is done by this plugin.

This is certainly connected to the behavior, that has been reported here. I'll let discussion progress a little more, but I suspect, that the sanest solutions will be to dump this silly quoting once and for all.

Changed 3 years ago by jun66j5

comment:3 Changed 3 years ago by jun66j5

If a query contains \' (backslash single-quote) or ' (single-quote), it seems Query class generates wrong tokens.

>>> from tractags.query import Query
>>> Query(r"'single\'quote'")
("single\'quote")
>>> Query(r"single'quote")
(and
  ("single")
  ("quote"))

I think that the class should generate ("single'quote") from the both.

TagsPlugin with tagsplugin-r10682.diff does as the following, [[ListTagged(single'quote)]] and /tags?q='single%5C'quote' works.

>>> from tractags.query import Query
>>> Query(r"'single\'quote'")
("single'quote")
>>> Query(r"single'quote")
("single'quote")

comment:4 Changed 3 years ago by hasienda

  • Status changed from new to assigned
  • Summary changed from Tags with single quote not tokenised correctly to [patch] Tags with single quote not tokenised correctly

Seems like a fairly elaborated fix. Even with test case, thank you very much. I'll check this.

comment:5 Changed 3 years ago by hasienda

(In [10784]) TagsPlugin: Correct tokenization for the 'or' operator, refs #3983 and #9057.

The regular expression introduced in [7378] is corrected now for tokenization to finally match expectations with respect to the particular behavior: All included tests pass now. Thanks to Jun Omae for the patch.

comment:6 in reply to: ↑ 2 Changed 3 years ago by hasienda

  • Cc rjollos otaku42 added; anonymous removed

Replying to hasienda:

While resolving other issues I've been stumbling over the rather strange quoting that is done by this plugin.

This is certainly connected to the behavior, that has been reported here. ![...]

Disagree now with myself, but or because I've found #3624 to be an issue clearly related to tag quoting in URLs.

Anyway, I'm glad Jun's patch provides the fix for the tokenization issue reported here, as I've verified now on my own.

comment:7 Changed 3 years ago by hasienda

(In [10785]) TagsPlugin: Correct tokenization for unary quotes, refs #3624 and #9057.

TagsQuery now has got a unit test in addition to the rather old doc tests. Thanks to Jun Omae for contributing most of the code to this changeset.

comment:8 follow-up: Changed 3 years ago by hasienda

Jun, would you comment on the changes your patch attempts on tractags/tests/__init__.py, please? I figure this could provide an alternative way to call the test suite, but can't figure out how exactly this should work. Tests are running fine without it, so this his really optional, right?

comment:9 in reply to: ↑ 8 ; follow-up: Changed 3 years ago by jun66j5

Replying to hasienda:

Jun, would you comment on the changes your patch attempts on tractags/tests/__init__.py, please? I figure this could provide an alternative way to call the test suite, but can't figure out how exactly this should work.

Hi Steffen,

If the tests with python tractags/tests/__init__.py, no tests run.

jun66j5@gotanda:745$ PYTHONPATH=$PWD ~/venv/trac-0.12/bin/python tractags/tests/__init__.py

With the patch;

jun66j5@gotanda:738$ PYTHONPATH=$PWD ~/venv/trac-0.12/bin/python tractags/tests/__init__.py
....................
----------------------------------------------------------------------
Ran 20 tests in 0.525s

OK

Tests are running fine without it, so this his really optional, right?

Right. I just understand that the tests run with setup.py test, now.

jun66j5@gotanda:742$ PYTHONPATH=$PWD ~/venv/trac-0.12/bin/python setup.py test
running test
running egg_info
writing requirements to TracTags.egg-info/requires.txt
... (snip)
test_matches (tractags.tests.web_ui.TagRequestHandlerTestCase) ... ok
test_matches_no_permission (tractags.tests.web_ui.TagRequestHandlerTestCase) ... ok
test_init (tractags.tests.wiki.WikiTagProviderTestCase) ... ok

----------------------------------------------------------------------
Ran 20 tests in 0.525s

OK

comment:10 Changed 3 years ago by hasienda

(In [10788]) TagsPlugin: Add an alternative way to start the test suite, refs #9057.

This is a leftover from Jun's patch and allows to start a full test run with minimal output by just calling tractags/tests/__init__.py.

comment:11 Changed 19 months ago by hasienda

(In [13134]) TagsPlugin: Remove query string for simple single-tag URLs, refs #9057.

As I noticed some time ago, this is actually work to remove a regression, because it has actually been the standard behavior before tags-0.6 too.

comment:12 in reply to: ↑ 9 Changed 9 months ago by hasienda

Replying to jun66j5:

Right. I just understand that the tests run with setup.py test, now.

Still holds for the current, greatly extended test stack:

$> PYTHONPATH=$PWD python tractags/tests/__init__.py
......................................................
----------------------------------------------------------------------
Ran 54 tests in 1.405s

OK

Btw, the unit test suite has been exceptionally useful in rapid testing for regressions during development towards [13730]. I'm still very grateful for osimons's start of unit tests for this plugin.

comment:13 Changed 9 months ago by hasienda

  • Resolution set to fixed
  • Status changed from assigned to closed

In 13815:

TagsPlugin: Completing preparation for v0.7 release.

Availability of that code as stable, tagged release
closes #2429, #3359, #3610, #3624, #3677, #3754, #3864, #3947, #3983, #4078, #4277, #4503, #4799, #5523, #7787, #7857, #8638, #9057, #9058, #9059, #9060, #9061, #9062, #9063, #9149, #9210, #9521, #9630, #9636, #10032, #10416, #10636, #11096, #11147, #11152, #11274, #11302, #11658 and #11659.

Additionally there are some issues and enhancement requests showing progress,
but known to require more work to resolve them satisfactorily, specifically
refs #2804, #4200, #8747 and #9064.

Thanks to all contributors and followers, that enabled and encouraged a good
portion of this development work.

Add Comment

Modify Ticket

Action
as closed The owner will remain hasienda.
The resolution will be deleted. Next status will be 'reopened'.
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.