Commit 563b57d6 authored by Terri Chu's avatar Terri Chu Committed by Bob Van Landuyt

Resolve "Quoted search term breaks code highlighting"

parent 60d07686
import $ from 'jquery'; import $ from 'jquery';
import setHighlightClass from 'ee_else_ce/search/highlight_blob_search_result';
import initDeprecatedJQueryDropdown from '~/deprecated_jquery_dropdown'; import initDeprecatedJQueryDropdown from '~/deprecated_jquery_dropdown';
import { deprecatedCreateFlash as Flash } from '~/flash'; import { deprecatedCreateFlash as Flash } from '~/flash';
import Api from '~/api'; import Api from '~/api';
...@@ -6,7 +7,6 @@ import { __ } from '~/locale'; ...@@ -6,7 +7,6 @@ import { __ } from '~/locale';
import Project from '~/pages/projects/project'; import Project from '~/pages/projects/project';
import { visitUrl } from '~/lib/utils/url_utility'; import { visitUrl } from '~/lib/utils/url_utility';
import refreshCounts from './refresh_counts'; import refreshCounts from './refresh_counts';
import setHighlightClass from './highlight_blob_search_result';
export default class Search { export default class Search {
constructor() { constructor() {
......
...@@ -7,4 +7,4 @@ ...@@ -7,4 +7,4 @@
= search_blob_title(project, path) = search_blob_title(project, path)
- if blob.data - if blob.data
.file-content.code.term{ data: { qa_selector: 'file_text_content' } } .file-content.code.term{ data: { qa_selector: 'file_text_content' } }
= render 'shared/file_highlight', blob: blob, first_line_number: blob.startline, blob_link: blob_link = render 'shared/file_highlight', blob: blob, first_line_number: blob.startline, blob_link: blob_link, highlight_line: blob.highlight_line
#blob-content.file-content.code.js-syntax-highlight #blob-content.file-content.code.js-syntax-highlight
- offset = defined?(first_line_number) ? first_line_number : 1
.line-numbers .line-numbers
- if blob.data.present? - if blob.data.present?
- link_icon = sprite_icon('link', size: 12) - link_icon = sprite_icon('link', size: 12)
- link = blob_link if defined?(blob_link) - link = blob_link if defined?(blob_link)
- blob.data.each_line.each_with_index do |_, index| - blob.data.each_line.each_with_index do |_, index|
- offset = defined?(first_line_number) ? first_line_number : 1
- i = index + offset - i = index + offset
-# We're not using `link_to` because it is too slow once we get to thousands of lines. -# We're not using `link_to` because it is too slow once we get to thousands of lines.
%a.diff-line-num{ href: "#{link}#L#{i}", id: "L#{i}", 'data-line-number' => i } %a.diff-line-num{ href: "#{link}#L#{i}", id: "L#{i}", 'data-line-number' => i }
= link_icon = link_icon
= i = i
.blob-content{ data: { blob_id: blob.id, path: blob.path } } - highlight = defined?(highlight_line) && highlight_line ? highlight_line - offset : nil
.blob-content{ data: { blob_id: blob.id, path: blob.path, highlight_line: highlight } }
%pre.code.highlight %pre.code.highlight
%code %code
= blob.present.highlight = blob.present.highlight
import setHighlightClass from '~/search/highlight_blob_search_result';
export default () => {
const highlightLineClass = 'hll';
const contentBody = document.getElementById('content-body');
const blobs = contentBody.querySelectorAll('.blob-result');
// Supports Basic (backed by Gitaly) Search highlighting
setHighlightClass();
// Supports Advanced (backed by Elasticsearch) Search highlighting
blobs.forEach(blob => {
const lines = blob.querySelectorAll('.line');
const dataHighlightLine = blob.querySelector('[data-highlight-line]');
if (dataHighlightLine) {
const { highlightLine } = dataHighlightLine.dataset;
lines[highlightLine].classList.add(highlightLineClass);
}
});
};
---
title: Fix quoted search term code highlighting
merge_request: 45278
author:
type: fixed
...@@ -142,10 +142,12 @@ module Gitlab ...@@ -142,10 +142,12 @@ module Gitlab
highlight_content = result.dig('highlight', 'blob.content')&.first || '' highlight_content = result.dig('highlight', 'blob.content')&.first || ''
found_line_number = 0 found_line_number = 0
highlight_found = false
highlight_content.each_line.each_with_index do |line, index| highlight_content.each_line.each_with_index do |line, index|
if line.include?(::Elastic::Latest::GitClassProxy::HIGHLIGHT_START_TAG) if line.include?(::Elastic::Latest::GitClassProxy::HIGHLIGHT_START_TAG)
found_line_number = index found_line_number = index
highlight_found = true
break break
end end
end end
...@@ -163,12 +165,15 @@ module Gitlab ...@@ -163,12 +165,15 @@ module Gitlab
end end
data = content.lines[from..to] data = content.lines[from..to]
# only send highlighted line number if a highlight was returned by Elasticsearch
highlight_line = highlight_found ? found_line_number + 1 : nil
::Gitlab::Search::FoundBlob.new( ::Gitlab::Search::FoundBlob.new(
path: path, path: path,
basename: basename, basename: basename,
ref: ref, ref: ref,
startline: from + 1, startline: from + 1,
highlight_line: highlight_line,
data: data.join, data: data.join,
project: project, project: project,
project_id: project_id project_id: project_id
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe SearchController, '(JavaScript fixtures)', type: :controller do
include JavaScriptFixturesHelpers
render_views
let_it_be(:user) { create(:admin) }
before(:all) do
clean_frontend_fixtures('ee/search/')
end
before do
sign_in(user)
end
context 'search within a project', :sidekiq_inline do
let(:namespace) { create(:namespace, name: 'frontend-fixtures') }
let(:project) { create(:project, :public, :repository, namespace: namespace, path: 'search-project') }
let(:blobs) do
Kaminari.paginate_array([
Gitlab::Search::FoundBlob.new(
path: 'CHANGELOG',
basename: 'CHANGELOG',
ref: 'master',
data: "hello\nworld\nfoo\nbar # this is the highlight\nbaz\nboo\nbat",
project: project,
project_id: project.id,
startline: 2,
highlight_line: 4),
Gitlab::Search::FoundBlob.new(
path: 'CONTRIBUTING',
basename: 'CONTRIBUTING',
ref: 'master',
data: "hello\nworld\nfoo\nbar # this is the highlight\nbaz\nboo\nbat",
project: project,
project_id: project.id,
startline: 2,
highlight_line: 4),
Gitlab::Search::FoundBlob.new(
path: 'README',
basename: 'README',
ref: 'master',
data: "foo\nbar # this is the highlight\nbaz\nboo\nbat",
project: project,
project_id: project.id,
startline: 2,
highlight_line: 2)
],
total_count: 3,
limit: 3,
offset: 0)
end
it 'ee/search/blob_search_result.html' do
expect_next_instance_of(SearchService) do |search_service|
expect(search_service).to receive(:search_objects).and_return(blobs)
end
get :show, params: {
search: 'Send',
project_id: project.id,
scope: :blobs
}
expect(response).to be_successful
end
end
end
import setHighlightClass from 'ee/search/highlight_blob_search_result';
const fixture = 'ee/search/blob_search_result.html';
const ceFixture = 'search/blob_search_result.html';
describe('ee/search/highlight_blob_search_result', () => {
preloadFixtures(fixture, ceFixture);
// Basic search support
it('highlights lines with search term occurrence', () => {
loadFixtures(ceFixture);
setHighlightClass();
expect(document.querySelectorAll('.blob-result .hll').length).toBe(4);
});
// Advanced search support
it('highlights lines which have been identified by Elasticsearch', () => {
loadFixtures(fixture);
setHighlightClass();
expect(document.querySelectorAll('.blob-result .hll').length).toBe(3);
});
});
...@@ -101,6 +101,7 @@ RSpec.describe Gitlab::Elastic::SearchResults, :elastic, :sidekiq_might_not_need ...@@ -101,6 +101,7 @@ RSpec.describe Gitlab::Elastic::SearchResults, :elastic, :sidekiq_might_not_need
expect(parsed).to be_kind_of(::Gitlab::Search::FoundBlob) expect(parsed).to be_kind_of(::Gitlab::Search::FoundBlob)
expect(parsed).to have_attributes( expect(parsed).to have_attributes(
startline: 1, startline: 1,
highlight_line: nil,
project: project, project: project,
data: "foo\n" data: "foo\n"
) )
...@@ -123,6 +124,7 @@ RSpec.describe Gitlab::Elastic::SearchResults, :elastic, :sidekiq_might_not_need ...@@ -123,6 +124,7 @@ RSpec.describe Gitlab::Elastic::SearchResults, :elastic, :sidekiq_might_not_need
basename: 'path/file', basename: 'path/file',
ref: 'sha', ref: 'sha',
startline: 2, startline: 2,
highlight_line: 2,
project: project, project: project,
data: "bar\n" data: "bar\n"
) )
...@@ -176,6 +178,7 @@ RSpec.describe Gitlab::Elastic::SearchResults, :elastic, :sidekiq_might_not_need ...@@ -176,6 +178,7 @@ RSpec.describe Gitlab::Elastic::SearchResults, :elastic, :sidekiq_might_not_need
basename: 'path/file', basename: 'path/file',
ref: 'sha', ref: 'sha',
startline: 2, startline: 2,
highlight_line: 4,
project: project, project: project,
data: expected_data data: expected_data
) )
......
...@@ -9,7 +9,7 @@ module Gitlab ...@@ -9,7 +9,7 @@ module Gitlab
include Gitlab::Utils::StrongMemoize include Gitlab::Utils::StrongMemoize
include BlobActiveModel include BlobActiveModel
attr_reader :project, :content_match, :blob_path attr_reader :project, :content_match, :blob_path, :highlight_line
PATH_REGEXP = /\A(?<ref>[^:]*):(?<path>[^\x00]*)\x00/.freeze PATH_REGEXP = /\A(?<ref>[^:]*):(?<path>[^\x00]*)\x00/.freeze
CONTENT_REGEXP = /^(?<ref>[^:]*):(?<path>[^\x00]*)\x00(?<startline>\d+)\x00/.freeze CONTENT_REGEXP = /^(?<ref>[^:]*):(?<path>[^\x00]*)\x00(?<startline>\d+)\x00/.freeze
...@@ -26,6 +26,7 @@ module Gitlab ...@@ -26,6 +26,7 @@ module Gitlab
@binary_basename = opts.fetch(:basename, nil) @binary_basename = opts.fetch(:basename, nil)
@ref = opts.fetch(:ref, nil) @ref = opts.fetch(:ref, nil)
@startline = opts.fetch(:startline, nil) @startline = opts.fetch(:startline, nil)
@highlight_line = opts.fetch(:highlight_line, nil)
@binary_data = opts.fetch(:data, nil) @binary_data = opts.fetch(:data, nil)
@per_page = opts.fetch(:per_page, 20) @per_page = opts.fetch(:per_page, 20)
@project = opts.fetch(:project, nil) @project = opts.fetch(:project, nil)
......
...@@ -26,8 +26,51 @@ RSpec.describe SearchController, '(JavaScript fixtures)', type: :controller do ...@@ -26,8 +26,51 @@ RSpec.describe SearchController, '(JavaScript fixtures)', type: :controller do
context 'search within a project' do context 'search within a project' do
let(:namespace) { create(:namespace, name: 'frontend-fixtures') } let(:namespace) { create(:namespace, name: 'frontend-fixtures') }
let(:project) { create(:project, :public, :repository, namespace: namespace, path: 'search-project') } let(:project) { create(:project, :public, :repository, namespace: namespace, path: 'search-project') }
let(:blobs) do
Kaminari.paginate_array([
Gitlab::Search::FoundBlob.new(
path: 'CHANGELOG',
basename: 'CHANGELOG',
ref: 'master',
data: "hello\nworld\nfoo\nSend # this is the highligh\nbaz\nboo\nbat",
project: project,
project_id: project.id,
startline: 2),
Gitlab::Search::FoundBlob.new(
path: 'CONTRIBUTING',
basename: 'CONTRIBUTING',
ref: 'master',
data: "hello\nworld\nfoo\nSend # this is the highligh\nbaz\nboo\nbat",
project: project,
project_id: project.id,
startline: 2),
Gitlab::Search::FoundBlob.new(
path: 'README',
basename: 'README',
ref: 'master',
data: "foo\nSend # this is the highlight\nbaz\nboo\nbat",
project: project,
project_id: project.id,
startline: 2),
Gitlab::Search::FoundBlob.new(
path: 'test',
basename: 'test',
ref: 'master',
data: "foo\nSend # this is the highlight\nbaz\nboo\nbat",
project: project,
project_id: project.id,
startline: 2)
],
total_count: 4,
limit: 4,
offset: 0)
end
it 'search/blob_search_result.html' do it 'search/blob_search_result.html' do
expect_next_instance_of(SearchService) do |search_service|
expect(search_service).to receive(:search_objects).and_return(blobs)
end
get :show, params: { get :show, params: {
search: 'Send', search: 'Send',
project_id: project.id, project_id: project.id,
......
import setHighlightClass from '~/pages/search/show/highlight_blob_search_result'; import setHighlightClass from '~/search/highlight_blob_search_result';
const fixture = 'search/blob_search_result.html'; const fixture = 'search/blob_search_result.html';
describe('pages/search/show/highlight_blob_search_result', () => { describe('search/highlight_blob_search_result', () => {
preloadFixtures(fixture); preloadFixtures(fixture);
beforeEach(() => loadFixtures(fixture)); beforeEach(() => loadFixtures(fixture));
...@@ -10,6 +10,6 @@ describe('pages/search/show/highlight_blob_search_result', () => { ...@@ -10,6 +10,6 @@ describe('pages/search/show/highlight_blob_search_result', () => {
it('highlights lines with search term occurrence', () => { it('highlights lines with search term occurrence', () => {
setHighlightClass(); setHighlightClass();
expect(document.querySelectorAll('.blob-result .hll').length).toBe(11); expect(document.querySelectorAll('.blob-result .hll').length).toBe(4);
}); });
}); });
import $ from 'jquery'; import $ from 'jquery';
import setHighlightClass from 'ee_else_ce/search/highlight_blob_search_result';
import Api from '~/api'; import Api from '~/api';
import Search from '~/pages/search/show/search'; import Search from '~/pages/search/show/search';
import setHighlightClass from '~/pages/search/show/highlight_blob_search_result';
jest.mock('~/api'); jest.mock('~/api');
jest.mock('~/pages/search/show/highlight_blob_search_result'); jest.mock('ee_else_ce/search/highlight_blob_search_result');
describe('Search', () => { describe('Search', () => {
const fixturePath = 'search/show.html'; const fixturePath = 'search/show.html';
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment