Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only match entities not embedded in links #3

Merged
merged 2 commits into from
Sep 30, 2024

Conversation

staab
Copy link
Collaborator

@staab staab commented Aug 26, 2024

Before this change, pasting njump links would trigger the entity regexp. This ensures only entities surrounded by a space or the beginning/end of a string will get matched.

@staab staab requested a review from cesardeazevedo August 26, 2024 23:33
@staab staab force-pushed the entity-word-boundaries branch from fd8d229 to 36c4f77 Compare August 26, 2024 23:59
@cesardeazevedo
Copy link
Owner

Yeah, this problem is very annoying, I also tried to do this exact approach you did but I ended up reverting it because of other issues it caused, I will do some more investigation before merging it.

@cesardeazevedo
Copy link
Owner

What do you think of

for (const match of text.matchAll(NEVENT_REGEX)) {
+  if (text[match.index - 1] !== '/') {
    try {
      const data = nip19.decode(match[2]).data as EventPointer
      const nevent = match[0]

      matches.push(createPasteRuleMatch(match, { ...data, nevent }))
    } catch (e) {
      continue
    }
+  }
}

@staab
Copy link
Collaborator Author

staab commented Aug 27, 2024

What kind of other issues did you run into? We could limit the scope of the regex ((?:\/) would be equivalent to the code you proposed above I think), but white-listing specific characters could turn into a game of whack-a-mole. If you want to go that route though, the only cases I can think of would be / and = in case the entity is in a query parameter.

@cesardeazevedo
Copy link
Owner

Pasting multiple nprofiles at once eats the space between them, doesn't happen with block nodes though

@staab
Copy link
Collaborator Author

staab commented Aug 29, 2024

Even if there's a space between the nprofiles in the copied text? Seems like a bug rather than a parser problem. I've run into lots of similar things working with tiptap/prosemirror. It's very hard to get the behavior exactly right.

@staab staab force-pushed the entity-word-boundaries branch from 36c4f77 to f82863e Compare August 29, 2024 16:13
@cesardeazevedo
Copy link
Owner

@staab Sorry for the delay, just coming back into this.

I just tried with (?<![\w./:?=]) and worked, with a lookbehind assertion the matched position stays the same.

Here's the issue with (?:^|\s)

pasting:

nostr:nprofile1qy88wumn8ghj7mn0wvhxcmmv9uq32amnwvaz7tmjv4kxz7fwv3sk6atn9e5k7tcprfmhxue69uhhyetvv9ujuem9w3skccne9e3k7mf0wccsqgxxvqas78x0a339m8qgkaf7fam5atmarne8dy3rzfd4l4x6w2qpncmfs8zh nostr:nprofile1qy88wumn8ghj7mn0wvhxcmmv9uq32amnwvaz7tmjv4kxz7fwv3sk6atn9e5k7tcprfmhxue69uhhyetvv9ujuem9w3skccne9e3k7mf0wccsqgxxvqas78x0a339m8qgkaf7fam5atmarne8dy3rzfd4l4x6w2qpncmfs8zh
Screenshot 2024-09-29 at 19 45 27

Now with (?<![\w./:?=])
Screenshot 2024-09-29 at 19 46 32

njump links
Screenshot 2024-09-29 at 20 05 13

Let me know what you think.

@staab staab force-pushed the entity-word-boundaries branch from f82863e to d0dc1d9 Compare September 30, 2024 17:39
@staab staab force-pushed the entity-word-boundaries branch from d0dc1d9 to c721c4a Compare September 30, 2024 17:39
@staab
Copy link
Collaborator Author

staab commented Sep 30, 2024

That makes sense, just pushed an update

@cesardeazevedo cesardeazevedo merged commit f47e625 into main Sep 30, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants