feat: clickable links in text fields #924
Conversation
CyanVoxel
added
Type: Enhancement
|
Confirmed working on Linux x86 and macOs ARM. Not sure if there is a way to do this but maybe you could make it so that I don't have to type "http(s)://" in front of my URLs? ex: "tagstud.io" isn't clickable, but "https://tagstud.io" is. |
|
Unfortunately, that's not exactly the easiest to do without creating a LOT of false positives. Requiring the protocol seems to be common for most apps/websites, so that's what I went with. |
one approach might be looking at the tld of the candidate domain, because which ones are valid is quite limited (there is a wikipedia page that lists all of them) |
|
Hm, that's true, though there's still the possibility of it running into false positives if the user happens to include a period anywhere in a text field, which isn't ideal |
|
definitely, but this kind of automatic detection in text always does. For that reason I personally favor manually marking things as links with markdown (having markdown WYSIWYG for notes is something that has been discussed before as well, not sure if there is commitment on that yet though), however I am not sure what would be best for the average joe (I write most documents in LaTeX or Markdown so I am not exactly representative of the average person in that regard) |
|
I guess I didn't realize this also does it in text fields but I was talking about the URL field specifically. Maybe we could have full protocol checking in regular text fields and no checking in URL fields? |
|
After looking into it a bit, it seems like there's over 1k TLDs that would have to be checked for, which.. isn't very feasible to check for |
This could be a good solution, though it would have to be explained to the user somehow, as some people probably aren't very familiar with markdown |
Hell, if going that route, there's the possibility of just going ahead and adding markdown rendering support to text fields. Though, that's a whole other can of worms. |
|
At least for now though, I think the current implementation is fine enough for the time being, unless any problems are found |
|
I don't think we have to go that far, just a regex (like this, for example: |
After checking it really quickly, it doesn't match the URL you're trying to match |
|
Yeah, I think going without protocol in text fields would add too many false positives, like you said. Putting the full protocol in text fields and not having to do it in URL fields feels like a good compromise |
|
In that case, I think a specific URL widget would have to be added, as currently the URL-related fields just use the text box widget |
|
In that case I'm just going to say it's good as is once the ruff errors are fixed. |
|
Thing is, the Ruff errors aren't related to my commit, they're from the source. Fixing them would mean editing random source code that's entirely unrelated to the feature itself, and should be left to a PR focused on fixing them. |
|
Ok nvm, apparently I misremembered |
| @@ -19,9 +21,22 @@ def __init__(self, title, text: str) -> None: | |||
| self.text_label = QLabel() | |||
| self.text_label.setStyleSheet("font-size: 12px") | |||
| self.text_label.setWordWrap(True) | |||
| self.text_label.setTextInteractionFlags(Qt.TextInteractionFlag.TextSelectableByMouse) | |||
| self.text_label.setTextFormat(Qt.TextFormat.RichText) | |||
There was a problem hiding this comment.
I would prefer setting this to markdown, as (afaik) the long term plan is to have WYSIWYG markdown editing for various fields. This would also mean changing the substitution further down.
(Or not setting it at all so that people don't get used to / have their libraries heavily use RichText Formatting)
src/tagstudio/qt/widgets/text.py
Outdated
|
|
||
| # Regex from https://stackoverflow.com/a/6041965 | ||
| def linkify(text: str): | ||
| url_pattern = r"(http|ftp|https):\/\/([\w_-]+(?:(?:\.[\w_-]+)+))([\w.,@?^=%&:\/~+#-*]*[\w@?^=%&\/~+#-*])" |
There was a problem hiding this comment.
Would recognising file:// be desirable as well? Because major browsers support that as well
There was a problem hiding this comment.
I think compiling this regex with re.compile() once and storing it as a global might make it quicker, thought ?
src/tagstudio/qt/widgets/text.py
Outdated
| lambda url: f'<a href="{url.group(0)}">{url.group(0)}</a>', | ||
| text, | ||
| flags=re.IGNORECASE | ||
| ) |
There was a problem hiding this comment.
Nit: Missing trailing new line
|
Disabled the Ruff "line too long" error (can't exactly add line breaks to the middle of a regex pattern) and added a trailing new line |
|
I think technically you can (see re.VERBOSE) but its fine here imo |
|
Turned out to be unnecessary regardless |
Summary
Text fields will now replace all* URLs in their text with clickable links. Text fields have also been changed to support links and be able to open them when clicked.
Tasks Completed
Closes #506