Problem/Motivation

The node title "Abc Xyz+" does not get replaced as term, as it seems. Could we have a test to check that and try fixing, if possible?

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Issue fork glossify-3538523

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

anybody created an issue. See original summary.

vinodhini.e made their first commit to this issue’s fork.

grevil made their first commit to this issue’s fork.

grevil opened merge request !53

grevil’s picture

grevil commented
Priority: Minor » Normal
Status: Active » Needs work

Ok this should do the trick! The problem was, that we use a word boundary at the end of the regex pattern ("/b"). Since "+" isn't considered a word character, the pattern can not match Abc Xyz+.

Drupal probably also allows entities to be named "+Abc Xyz" as well? I guess that will also fail because of the leading "/b". I will provide a test and a fix tommorow.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

Thanks @grevil. We have to be careful then, because maybe the change will lead to issues where partial words are replaced, which might be undeserved. Maybe we'll need a setting, if we don''t find a good general solution. Thanks for finding out the reason so far!

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

Maybe we should first add an issue to add tests for not breaking partial words. After merging that and updating the fork, I guess they will fail expectedly.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Title: "Abc Xyz+" does not get replaced » Add option for partial word glossification ("Abc Xyz+" does not get replaced)
grevil’s picture

grevil commented
Related issues: +#3538691: Add tests to check, that words partially containing an entity name won't get glossified and fix deprecations

Now that #3538691: Add tests to check, that words partially containing an entity name won't get glossified and fix deprecations is fixed, let's see if the tests still succeed.

grevil’s picture

grevil commented

Nice the test fails now!

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

Nice! So let's add the 4-options select option for "Partial word match":

  • Only match whole words (default)
  • Match partially at end of words
  • Match partially at beginning of words
  • Match partially anywhere
grevil’s picture

grevil commented

Nice! So let's add the 4-options select option for "Partial word match":

Only match whole words (default)
Match partially at end of words
Match partially at beginning of words
Match partially anywhere

I don't think, that is related to this issue, but we can create a follow-up issue for that in the future.

grevil’s picture

grevil commented
Assigned: grevil » Unassigned
Status: Needs work » Needs review

The current approach should be the option we have to fix this issue.

It will have problems with terms like " test ", "test." or "test," but I don't think it is even possible to save drupal entities having a label with a trailing whitespace? And using "." or "," inside the label is also super uncommon and would have let to a problem before.

grevil’s picture

grevil commented
Status: Needs review » Reviewed & tested by the community

Regarding the many tests we have, this fix is fairly "save". I'll merge it.

  • grevil committed 49d78a5d on 3.x 
    Issue #3538523: "Abc Xyz+" does not get replaced
grevil’s picture

grevil commented
Status: Reviewed & tested by the community » Fixed

Now that this issue is closed, please review the contribution record.

As a contributor, attribute any organization helped you, or if you volunteered your own time.

Maintainers, please credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.