-
Notifications
You must be signed in to change notification settings - Fork 3.2k
Accept semantic work-package identifiers in CKEditor text macros #22976
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
Changes from 33 commits
f2a2ab0
2e92308
1666ccd
06a5d61
74c12f9
1e7a649
085ed19
dd42232
f83cbf9
53986c0
4566337
06d7e64
013b6e1
90a3c5b
4090ea2
d1b87a6
1e59f69
c066978
b58cbfb
6aa3699
6b31bf6
b8dfb39
7647174
8728c0e
9ef7627
3de326f
d5a8c5d
5fda19b
fe9bfe3
6302027
366588c
443cc7c
e51ccd1
26975ee
9339d84
5df1456
9001986
0fd9f44
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -31,8 +31,21 @@ | |
| module WorkPackage::Exports | ||
| module Macros | ||
| class WorkPackagesLinkHandler < OpenProject::TextFormatting::Matchers::LinkHandlers::WorkPackages | ||
| # PDF export only handles canonical numeric `#N` references. Semantic and | ||
| # leading-zero shapes fall through to literal text to avoid emitting a | ||
| # `<mention data-id="0">` (since `"PROJ-1".to_i == 0`). Semantic-id | ||
| # support in PDF export is tracked in https://community.openproject.org/wp/74766. | ||
| def applicable? | ||
|
Comment on lines
+34
to
38
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will be handled in #23138 |
||
| %w(# ## ###).include?(matcher.sep) && matcher.prefix.blank? | ||
| hash_trigger? && | ||
| matcher.prefix.blank? && | ||
| WorkPackage::SemanticIdentifier.numeric_id?(matcher.identifier) | ||
| end | ||
|
|
||
| # PDF rendering walks Markly nodes directly and bypasses the | ||
| # `PatternMatcherFilter` preload pipeline, so the parent's cache-driven | ||
| # `call` would miss every reference. | ||
| def call | ||
| render_link(matcher.identifier.to_i, matcher) | ||
| end | ||
|
|
||
| def render_link(wp_id, matcher) | ||
|
|
||
Large diffs are not rendered by default.
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -31,9 +31,6 @@ | |
| module OpenProject::TextFormatting | ||
| module Filters | ||
| class PatternMatcherFilter < HTML::Pipeline::Filter | ||
| # Skip text nodes that are within preformatted blocks | ||
| PREFORMATTED_BLOCKS = %w(pre code).to_set | ||
|
|
||
| class << self | ||
| def append_matcher(matcher) | ||
| matchers << matcher | ||
|
|
@@ -45,15 +42,35 @@ def matchers | |
| end | ||
|
|
||
| def call | ||
| with_matcher_preloads(self.class.matchers.dup) { process_text_nodes } | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See -> Text formatting cache flow interactive walkthrough How PatternMatcherFilter#with_matcher_preloads wraps ResourceLinksMatcher.with_preloaded_resources to build the per-render WP cache, then tears it down.
|
||
| doc | ||
| end | ||
|
|
||
| private | ||
|
|
||
| # Wraps the per-node loop in each matcher's `with_preloaded_resources` | ||
| # hook so matchers can warm per-render caches and save/restore them | ||
| # around nested `format_text` calls. Opt-in: matchers without the hook | ||
| # are skipped. | ||
| def with_matcher_preloads(matchers, &) | ||
| matcher = matchers.shift | ||
| return yield if matcher.nil? | ||
|
|
||
| if matcher.respond_to?(:with_preloaded_resources) | ||
| matcher.with_preloaded_resources(doc, context) { with_matcher_preloads(matchers, &) } | ||
| else | ||
| with_matcher_preloads(matchers, &) | ||
| end | ||
| end | ||
|
|
||
| def process_text_nodes | ||
| doc.search(".//text()").each do |node| | ||
| next if has_ancestor?(node, PREFORMATTED_BLOCKS) | ||
| next if has_ancestor?(node, OpenProject::TextFormatting::PreformattedBlocks::BLOCKS) | ||
|
|
||
| self.class.matchers.each do |matcher| | ||
| matcher.call(node, doc:, context:) | ||
| end | ||
| end | ||
|
|
||
| doc | ||
| end | ||
| end | ||
| end | ||
|
|
||

There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@akabiru Would you be open to split these parts into a separate refactoring PR? That way, I can already make use of the regexes here. 🙏
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure thing, I'll take a stab at that 👍🏾