Fix display of ellipsis in search fragments (#1896)

Display ellipsis as an indicator that there is more content before or behind a search result fragment only if there really is more content.
This commit is contained in:
Matthias Thieroff
2021-12-15 15:07:46 +01:00
committed by GitHub
parent 1118ddd146
commit 11673e6d07
11 changed files with 135 additions and 50 deletions

View File

@@ -0,0 +1,2 @@
- type: fixed
description: Do not display ellipsis if search result matches start or end of content ([#1896](https://github.com/scm-manager/scm-manager/pull/1896))

View File

@@ -81,7 +81,7 @@ public class Hit {
@Getter @Getter
@AllArgsConstructor(access = AccessLevel.PRIVATE) @AllArgsConstructor(access = AccessLevel.PRIVATE)
public abstract static class Field { public abstract static class Field {
boolean highlighted; private final boolean highlighted;
} }
/** /**
@@ -89,7 +89,7 @@ public class Hit {
*/ */
@Getter @Getter
public static class ValueField extends Field { public static class ValueField extends Field {
Object value; private final Object value;
public ValueField(Object value) { public ValueField(Object value) {
super(false); super(false);
@@ -102,11 +102,30 @@ public class Hit {
*/ */
@Getter @Getter
public static class HighlightedField extends Field { public static class HighlightedField extends Field {
String[] fragments; private final String[] fragments;
/**
* @since 2.28.0
*/
private final boolean matchesContentStart;
/**
* @since 2.28.0
*/
private final boolean matchesContentEnd;
public HighlightedField(String[] fragments) { public HighlightedField(String[] fragments) {
this(fragments, false, false);
}
/**
* @since 2.28.0
*/
public HighlightedField(String[] fragments, boolean matchesContentStart, boolean matchesContentEnd) {
super(true); super(true);
this.fragments = fragments; this.fragments = fragments;
this.matchesContentStart = matchesContentStart;
this.matchesContentEnd = matchesContentEnd;
} }
} }

View File

@@ -33,7 +33,9 @@ export const javaHit: Hit = {
"import org.slf4j.LoggerFactory;\n\nimport java.util.Date;\nimport java.util.HashMap;\nimport java.util.Map;\nimport java.util.concurrent.TimeUnit;\n\n/**\n * Jwt implementation of {@link <|[[--AccessTokenBuilder--]]|>}.\n * \n * @author Sebastian Sdorra\n * @since 2.0.0\n */\npublic final class <|[[--JwtAccessTokenBuilder--]]|> implements <|[[--AccessTokenBuilder--]]|> {\n\n /**\n * the logger for <|[[--JwtAccessTokenBuilder--]]|>\n */\n private static final Logger LOG = LoggerFactory.getLogger(<|[[--JwtAccessTokenBuilder.class--]]|>);\n \n private final KeyGenerator keyGenerator; \n private final SecureKeyResolver keyResolver; \n \n private String subject;\n private String issuer;\n", "import org.slf4j.LoggerFactory;\n\nimport java.util.Date;\nimport java.util.HashMap;\nimport java.util.Map;\nimport java.util.concurrent.TimeUnit;\n\n/**\n * Jwt implementation of {@link <|[[--AccessTokenBuilder--]]|>}.\n * \n * @author Sebastian Sdorra\n * @since 2.0.0\n */\npublic final class <|[[--JwtAccessTokenBuilder--]]|> implements <|[[--AccessTokenBuilder--]]|> {\n\n /**\n * the logger for <|[[--JwtAccessTokenBuilder--]]|>\n */\n private static final Logger LOG = LoggerFactory.getLogger(<|[[--JwtAccessTokenBuilder.class--]]|>);\n \n private final KeyGenerator keyGenerator; \n private final SecureKeyResolver keyResolver; \n \n private String subject;\n private String issuer;\n",
" private final Map<String,Object> custom = Maps.newHashMap();\n \n <|[[--JwtAccessTokenBuilder--]]|>(KeyGenerator keyGenerator, SecureKeyResolver keyResolver) {\n this.keyGenerator = keyGenerator;\n this.keyResolver = keyResolver;\n }\n\n @Override\n public <|[[--JwtAccessTokenBuilder--]]|> subject(String subject) {\n", " private final Map<String,Object> custom = Maps.newHashMap();\n \n <|[[--JwtAccessTokenBuilder--]]|>(KeyGenerator keyGenerator, SecureKeyResolver keyResolver) {\n this.keyGenerator = keyGenerator;\n this.keyResolver = keyResolver;\n }\n\n @Override\n public <|[[--JwtAccessTokenBuilder--]]|> subject(String subject) {\n",
' public <|[[--JwtAccessTokenBuilder--]]|> custom(String key, Object value) {\n Preconditions.checkArgument(!Strings.isNullOrEmpty(key), "null or empty value not allowed");\n Preconditions.checkArgument(value != null, "null or empty value not allowed");\n' ' public <|[[--JwtAccessTokenBuilder--]]|> custom(String key, Object value) {\n Preconditions.checkArgument(!Strings.isNullOrEmpty(key), "null or empty value not allowed");\n Preconditions.checkArgument(value != null, "null or empty value not allowed");\n'
] ],
matchesContentStart: false,
matchesContentEnd: false
} }
}, },
_links: {} _links: {}
@@ -46,7 +48,9 @@ export const bashHit: Hit = {
highlighted: true, highlighted: true,
fragments: [ fragments: [
'# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,\n# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE\n# SOFTWARE.\n#\n\n<|[[--getent--]]|> group scm >/dev/null || groupadd -r scm\n<|[[--getent--]]|> passwd scm >/dev/null || \\\n useradd -r -g scm -M \\\n -s /sbin/nologin -d /var/lib/scm \\\n -c "user for the scm-server process" scm\nexit 0\n\n' '# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,\n# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE\n# SOFTWARE.\n#\n\n<|[[--getent--]]|> group scm >/dev/null || groupadd -r scm\n<|[[--getent--]]|> passwd scm >/dev/null || \\\n useradd -r -g scm -M \\\n -s /sbin/nologin -d /var/lib/scm \\\n -c "user for the scm-server process" scm\nexit 0\n\n'
] ],
matchesContentStart: false,
matchesContentEnd: true
} }
}, },
_links: {} _links: {}
@@ -60,7 +64,9 @@ export const markdownHit: Hit = {
fragments: [ fragments: [
"---\ntitle: SCM-Manager v2 Test <|[[--Cases--]]|>\n---\n\nDescribes the expected behaviour for SCMM v2 REST Resources using manual tests.\n\nThe following states general test <|[[--cases--]]|> per HTTP Method and en expected return code as well as exemplary curl calls.\nResource-specifics are stated \n\n## Test <|[[--Cases--]]|>\n\n### GET\n\n- Collection Resource (e.g. `/users`)\n - Without parameters -> 200\n - Parameters\n - `?pageSize=1` -> Only one embedded element, pageTotal reflects the correct number of pages, `last` link points to last page.\n - `?pageSize=1&page=1` -> `next` link points to page 0 ; `prev` link points to page 2\n - `?sortBy=admin` -> Sorted by `admin` field of embedded objects\n - `?sortBy=admin&desc=true` -> Invert sorting\n- Individual Resource (e.g. `/users/scmadmin`)\n - Exists -> 200\n", "---\ntitle: SCM-Manager v2 Test <|[[--Cases--]]|>\n---\n\nDescribes the expected behaviour for SCMM v2 REST Resources using manual tests.\n\nThe following states general test <|[[--cases--]]|> per HTTP Method and en expected return code as well as exemplary curl calls.\nResource-specifics are stated \n\n## Test <|[[--Cases--]]|>\n\n### GET\n\n- Collection Resource (e.g. `/users`)\n - Without parameters -> 200\n - Parameters\n - `?pageSize=1` -> Only one embedded element, pageTotal reflects the correct number of pages, `last` link points to last page.\n - `?pageSize=1&page=1` -> `next` link points to page 0 ; `prev` link points to page 2\n - `?sortBy=admin` -> Sorted by `admin` field of embedded objects\n - `?sortBy=admin&desc=true` -> Invert sorting\n- Individual Resource (e.g. `/users/scmadmin`)\n - Exists -> 200\n",
"\n### DELETE\n\n- existing -> 204\n- not existing -> 204\n- without permission -> 401\n\n## Exemplary calls & Resource specific test <|[[--cases--]]|>\n\nIn order to extend those tests to other Resources, have a look at the rest docs. Note that the Content Type is specific to each resource as well.\n" "\n### DELETE\n\n- existing -> 204\n- not existing -> 204\n- without permission -> 401\n\n## Exemplary calls & Resource specific test <|[[--cases--]]|>\n\nIn order to extend those tests to other Resources, have a look at the rest docs. Note that the Content Type is specific to each resource as well.\n"
] ],
matchesContentStart: true,
matchesContentEnd: false
} }
}, },
_links: {} _links: {}

View File

@@ -90029,8 +90029,6 @@ exports[`Storyshots TextHitField Bash SyntaxHighlighting 1`] = `
exit 0 exit 0
...
</pre> </pre>
`; `;
@@ -90202,8 +90200,6 @@ public final class
exports[`Storyshots TextHitField Markdown SyntaxHighlighting 1`] = ` exports[`Storyshots TextHitField Markdown SyntaxHighlighting 1`] = `
<pre> <pre>
...
--- ---
title: SCM-Manager v2 Test title: SCM-Manager v2 Test
<mark> <mark>
@@ -90239,8 +90235,6 @@ Resource-specifics are stated
- Individual Resource (e.g. \`/users/scmadmin\`) - Individual Resource (e.g. \`/users/scmadmin\`)
- Exists -&gt; 200 - Exists -&gt; 200
...
### DELETE ### DELETE
@@ -90286,8 +90280,6 @@ exports[`Storyshots TextHitField Unknown SyntaxHighlighting 1`] = `
exit 0 exit 0
...
</pre> </pre>
`; `;

View File

@@ -39,13 +39,13 @@ const HighlightedTextField: FC<HighlightedTextFieldProps> = ({ field, syntaxHigh
<> <>
{field.fragments.map((fragment, i) => ( {field.fragments.map((fragment, i) => (
<React.Fragment key={fragment}> <React.Fragment key={fragment}>
{separator} {field.matchesContentStart ? null : separator}
{syntaxHighlightingLanguage ? ( {syntaxHighlightingLanguage ? (
<SyntaxHighlightedFragment value={fragment} language={syntaxHighlightingLanguage} /> <SyntaxHighlightedFragment value={fragment} language={syntaxHighlightingLanguage} />
) : ( ) : (
<HighlightedFragment value={fragment} /> <HighlightedFragment value={fragment} />
)} )}
{i + 1 >= field.fragments.length ? separator : null} {i + 1 >= field.fragments.length && !field.matchesContentEnd ? separator : null}
</React.Fragment> </React.Fragment>
))} ))}
</> </>

View File

@@ -140,6 +140,7 @@ module.exports = [
} }
}, },
historyApiFallback: true, historyApiFallback: true,
host: "127.0.0.1",
port: 3000, port: 3000,
hot: true, hot: true,
devMiddleware: { devMiddleware: {

View File

@@ -33,6 +33,8 @@ export type ValueHitField = {
export type HighlightedHitField = { export type HighlightedHitField = {
highlighted: true; highlighted: true;
fragments: string[]; fragments: string[];
matchesContentStart: boolean;
matchesContentEnd: boolean;
}; };
export type HitField = ValueHitField | HighlightedHitField; export type HitField = ValueHitField | HighlightedHitField;

View File

@@ -0,0 +1,44 @@
/*
* MIT License
*
* Copyright (c) 2020-present Cloudogu GmbH and Contributors
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in all
* copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
* SOFTWARE.
*/
package sonia.scm.search;
import lombok.Getter;
@Getter
public class ContentFragment {
private final String fragment;
private final boolean matchesContentStart;
private final boolean matchesContentEnd;
ContentFragment(String fragment) {
this(fragment, false, false);
}
ContentFragment(String fragment, boolean matchesContentStart, boolean matchesContentEnd) {
this.fragment = fragment;
this.matchesContentStart = matchesContentStart;
this.matchesContentEnd = matchesContentEnd;
}
}

View File

@@ -70,25 +70,32 @@ public final class LuceneHighlighter {
return field.isHighlighted() && queriedFields.contains(field.getName()); return field.isHighlighted() && queriedFields.contains(field.getName());
} }
public String[] highlight(String fieldName, Indexed.Analyzer fieldAnalyzer, String value) throws InvalidTokenOffsetsException, IOException { public ContentFragment[] highlight(String fieldName, Indexed.Analyzer fieldAnalyzer, String value) throws InvalidTokenOffsetsException, IOException {
String[] fragments = highlighter.getBestFragments(analyzer, fieldName, value, MAX_NUM_FRAGMENTS); String[] fragments = highlighter.getBestFragments(analyzer, fieldName, value, MAX_NUM_FRAGMENTS);
if (fieldAnalyzer == Indexed.Analyzer.CODE) { if (fieldAnalyzer == Indexed.Analyzer.CODE) {
fragments = keepWholeLine(value, fragments); return keepWholeLine(value, fragments);
} }
return Arrays.stream(fragments) return Arrays.stream(fragments).map(ContentFragment::new)
.toArray(String[]::new); .toArray(ContentFragment[]::new);
} }
private String[] keepWholeLine(String content, String[] fragments) { private ContentFragment[] keepWholeLine(String content, String[] fragments) {
return Arrays.stream(fragments) return Arrays.stream(fragments)
.map(fragment -> keepWholeLine(content, fragment)) .map(fragment -> keepWholeLine(content, fragment))
.toArray(String[]::new); .toArray(ContentFragment[]::new);
} }
private String keepWholeLine(String content, String fragment) { private ContentFragment keepWholeLine(String content, String fragment) {
boolean matchesContentStart = false;
boolean matchesContentEnd = false;
String raw = fragment.replace(PRE_TAG, "").replace(POST_TAG, ""); String raw = fragment.replace(PRE_TAG, "").replace(POST_TAG, "");
int index = content.indexOf(raw); int index = content.indexOf(raw);
if (index == 0) {
matchesContentStart = true;
}
int start = content.lastIndexOf('\n', index); int start = content.lastIndexOf('\n', index);
String snippet; String snippet;
@@ -109,9 +116,10 @@ public final class LuceneHighlighter {
int end = content.indexOf('\n', index + raw.length()); int end = content.indexOf('\n', index + raw.length());
if (end < 0) { if (end < 0) {
end = content.length(); end = content.length();
matchesContentEnd = true;
} }
return snippet + content.substring(index + raw.length(), end) + "\n"; return new ContentFragment(snippet + content.substring(index + raw.length(), end) + (matchesContentEnd ? "" : "\n"), matchesContentStart, matchesContentEnd);
} }
} }

View File

@@ -34,6 +34,7 @@ import org.apache.lucene.search.highlight.InvalidTokenOffsetsException;
import java.io.IOException; import java.io.IOException;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap; import java.util.HashMap;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
@@ -75,9 +76,11 @@ public class QueryResultFactory {
Object value = field.value(document); Object value = field.value(document);
if (value != null) { if (value != null) {
if (highlighter.isHighlightable(field)) { if (highlighter.isHighlightable(field)) {
String[] fragments = createFragments(field, value.toString()); ContentFragment[] fragments = createFragments(field, value.toString());
if (fragments.length > 0) { if (fragments.length > 0) {
return of(new Hit.HighlightedField(fragments)); boolean firstFragmentMatchesContentStart = fragments[0].isMatchesContentStart();
boolean lastFragmentMatchesContentEnd = fragments[fragments.length - 1].isMatchesContentEnd();
return of(new Hit.HighlightedField(Arrays.stream(fragments).map(ContentFragment::getFragment).toArray(String[]::new), firstFragmentMatchesContentStart, lastFragmentMatchesContentEnd));
} }
} }
return of(new Hit.ValueField(value)); return of(new Hit.ValueField(value));
@@ -85,7 +88,7 @@ public class QueryResultFactory {
return empty(); return empty();
} }
private String[] createFragments(LuceneSearchableField field, String value) throws InvalidTokenOffsetsException, IOException { private ContentFragment[] createFragments(LuceneSearchableField field, String value) throws InvalidTokenOffsetsException, IOException {
return highlighter.highlight(field.getName(), field.getAnalyzer(), value); return highlighter.highlight(field.getName(), field.getAnalyzer(), value);
} }

View File

@@ -56,43 +56,47 @@ class LuceneHighlighterTest {
String content = content("content"); String content = content("content");
LuceneHighlighter highlighter = new LuceneHighlighter(analyzer, query); LuceneHighlighter highlighter = new LuceneHighlighter(analyzer, query);
String[] snippets = highlighter.highlight("content", Indexed.Analyzer.DEFAULT, content); ContentFragment[] contentFragments = highlighter.highlight("content", Indexed.Analyzer.DEFAULT, content);
assertThat(snippets).hasSize(1).allSatisfy( assertThat(contentFragments).hasSize(1).allSatisfy(
snippet -> assertThat(snippet).contains("<|[[--Golgafrinchan--]]|>") contentFragment -> assertThat(contentFragment.getFragment()).contains("<|[[--Golgafrinchan--]]|>")
); );
} }
@Test @Test
void shouldHighlightCodeAndKeepLines() throws IOException, InvalidTokenOffsetsException { void shouldHighlightCodeAndKeepLines() throws IOException, InvalidTokenOffsetsException {
String[] snippets = highlightCode("GameOfLife.java", "die"); ContentFragment[] contentFragments = highlightCode("GameOfLife.java", "die");
assertThat(snippets).hasSize(1).allSatisfy( assertThat(contentFragments).hasSize(1).allSatisfy(
snippet -> assertThat(snippet.split("\n")).contains( contentFragment -> {
"\t\t\t\tint neighbors= getNeighbors(above, same, below);", assertThat(contentFragment.getFragment().split("\n")).contains(
"\t\t\t\tif(neighbors < 2 || neighbors > 3){", "\t\t\t\tint neighbors= getNeighbors(above, same, below);",
"\t\t\t\t\tnewGen[row]+= \"_\";//<2 or >3 neighbors -> <|[[--die--]]|>", "\t\t\t\tif(neighbors < 2 || neighbors > 3){",
"\t\t\t\t}else if(neighbors == 3){", "\t\t\t\t\tnewGen[row]+= \"_\";//<2 or >3 neighbors -> <|[[--die--]]|>",
"\t\t\t\t\tnewGen[row]+= \"#\";//3 neighbors -> spawn/live" "\t\t\t\t}else if(neighbors == 3){",
) "\t\t\t\t\tnewGen[row]+= \"#\";//3 neighbors -> spawn/live"
);
assertThat(contentFragment.isMatchesContentEnd()).isFalse();
assertThat(contentFragment.isMatchesContentEnd()).isFalse();
}
); );
} }
@Test @Test
void shouldNotStartHighlightedFragmentWithLineBreak() throws IOException, InvalidTokenOffsetsException { void shouldNotStartHighlightedFragmentWithLineBreak() throws IOException, InvalidTokenOffsetsException {
String[] snippets = highlightCode("GameOfLife.java", "die"); ContentFragment[] contentFragments = highlightCode("GameOfLife.java", "die");
assertThat(snippets).hasSize(1).allSatisfy( assertThat(contentFragments).hasSize(1).allSatisfy(
snippet -> assertThat(snippet).doesNotStartWith("\n") contentFragment -> assertThat(contentFragment.getFragment()).doesNotStartWith("\n")
); );
} }
@Test @Test
void shouldHighlightCodeInTsx() throws IOException, InvalidTokenOffsetsException { void shouldHighlightCodeInTsx() throws IOException, InvalidTokenOffsetsException {
String[] snippets = highlightCode("Button.tsx", "inherit"); ContentFragment[] contentFragments = highlightCode("Button.tsx", "inherit");
assertThat(snippets).hasSize(1).allSatisfy( assertThat(contentFragments).hasSize(1).allSatisfy(
snippet -> assertThat(snippet.split("\n")).contains( contentFragment -> assertThat(contentFragment.getFragment().split("\n")).contains(
"}) => {", "}) => {",
" const renderIcon = () => {", " const renderIcon = () => {",
" return <>{icon ? <Icon name={icon} color=\"<|[[--inherit--]]|>\" className=\"is-medium pr-1\" /> : null}</>;", " return <>{icon ? <Icon name={icon} color=\"<|[[--inherit--]]|>\" className=\"is-medium pr-1\" /> : null}</>;",
@@ -103,16 +107,20 @@ class LuceneHighlighterTest {
@Test @Test
void shouldHighlightFirstCodeLine() throws InvalidTokenOffsetsException, IOException { void shouldHighlightFirstCodeLine() throws InvalidTokenOffsetsException, IOException {
String[] snippets = highlightCode("GameOfLife.java", "gameoflife"); ContentFragment[] contentFragments = highlightCode("GameOfLife.java", "gameoflife");
assertThat(snippets).hasSize(1); assertThat(contentFragments).hasSize(1);
assertThat(contentFragments[0].isMatchesContentStart()).isTrue();
assertThat(contentFragments[0].isMatchesContentEnd()).isFalse();
} }
@Test @Test
void shouldHighlightLastCodeLine() throws InvalidTokenOffsetsException, IOException { void shouldHighlightLastCodeLine() throws InvalidTokenOffsetsException, IOException {
String[] snippets = highlightCode("Button.tsx", "default"); ContentFragment[] contentFragments = highlightCode("Button.tsx", "default");
assertThat(snippets).hasSize(1); assertThat(contentFragments).hasSize(1);
assertThat(contentFragments[0].isMatchesContentStart()).isFalse();
assertThat(contentFragments[0].isMatchesContentEnd()).isTrue();
} }
@Nested @Nested
@@ -154,7 +162,7 @@ class LuceneHighlighterTest {
} }
private String[] highlightCode(String resource, String search) throws IOException, InvalidTokenOffsetsException { private ContentFragment[] highlightCode(String resource, String search) throws IOException, InvalidTokenOffsetsException {
NonNaturalLanguageAnalyzer analyzer = new NonNaturalLanguageAnalyzer(); NonNaturalLanguageAnalyzer analyzer = new NonNaturalLanguageAnalyzer();
Query query = new TermQuery(new Term("content", search)); Query query = new TermQuery(new Term("content", search));