Fold links to files in GFM
What does this MR do?
Fold all URLs that reference a tree or a blob into a more user friendly format.
Are there points in the code the reviewer needs to double check?
Why was this MR needed?
To show URL references in a nicer way.
Screenshots (if relevant)
Does this MR meet the acceptance criteria?
-
CHANGELOG entry added - Tests
-
Added for this feature/bug -
All builds are passing
-
-
Conform by the style guides -
Branch has no merge conflicts with master
(if you do - rebase it please) -
Squashed related commits together
What are the relevant issue numbers?
Closes #14225 (moved)
Merge request reports
Activity
Marked the task CHANGELOG entry added as completed
Marked the task Squashed related commits together as completed
Marked the task Conform by the style guides as completed
- Resolved by username-removed-443319
- lib/banzai/filter/blob_link_filter.rb 0 → 100644
14 end 15 16 doc 17 end 18 19 protected 20 21 def process_link(node) 22 href = node.attribute('href').value 23 24 id = href.match(LINK_PATTERN).captures[2] 25 target_project = href.match(%r{#{internal_url}/(\S+)/(blob|tree)/(?<!,|\.)}).captures[0] 26 27 current_project = context[:project] 28 29 @project = Project::find_with_namespace(target_project) I didn't know this before, but there is a redactor class to take care of this! https://gitlab.com/gitlab-org/gitlab-ce/blob/master/lib/banzai/redactor.rb
I apologise for not knowing this sooner, but maybe this would be better if it worked more like the
CommitReferenceFilter
and its correspondingCommitParser
?They are similar in that they reference objects in the repository, rather than DB objects, but still need to be redacted for projects the user doesn't have access to.
This will end up running a database query for every link, which is not going to scale well. It's a better idea to try and extract the project from the URL instead. Also, never use
::
for method calls; use.
instead.Edited by yorickpeterse-staging
- Resolved by username-removed-682068
- Resolved by username-removed-682068
- Resolved by username-removed-682068
Thanks @SvenDub, this looks great!
🎉 I think there are some cases that have been missed, but things are in good shape so far.Added 1 commit:
- 4ce989a1 - Check for the existence of a project
- Resolved by username-removed-682068
@SvenDub do you think you'll be able to work on this any more? If not, let me know and someone can take it over (you'll still get credit). It would be great to have this feature!
@smcgivern Certainly! I wasn't sure what to do with this and lost it out of sight for a bit. Should I check for the existence of a project and run the database query as is the case now, or should I generate the link anyway?
@SvenDub sorry, I lost track of this again
😞 I think it would be interesting to treat this like commits, like I mentioned. There are differences, but they aren't insurmountable:
- We don't want to actually load the
Blob
when it's been referred to, because that would be horrific. I think we can assume it exists ifextract_ref
returns something sensible. We could either add aBlob.from_reference
class, or create a specialBlobReference
class. The latter might be clearer as it would share very little functionality with the 'real'Blob
, instead just mixing in and implementing methods fromReferable
. - We don't want to round-trip these. This can be resolved by making the
reference_pattern
class method onBlob[Reference]
returnnil
.
This gives us the reference redaction 'for free' in the case where the project doesn't exist, and we can assume that the existing reference redaction etc. code is reasonably performant. This is all a bit high level, but let me know what you think and I can point you in the right direction
🙂 - We don't want to actually load the
added ~874211 ~480950 ~159098 markdown labels