From 1a34156ab385cb0bd12fab36f100c6b3e7cb9701 Mon Sep 17 00:00:00 2001 From: Chris Fraire Date: Sat, 21 Mar 2020 14:55:46 -0500 Subject: [PATCH] Optionally show surrounding context during search Also: * Use tabindex="0" for Accessibility * Fix up print.css * Scale moreLimit with additional context but limited * Don't re-report scopes when showing surrounding context * Highlight match when surrounding context is shown --- .../org/opengrok/indexer/search/Results.java | 18 +- .../indexer/search/context/Context.java | 38 ++-- .../indexer/search/context/ContextArgs.java | 7 + .../search/context/ContextFormatter.java | 164 ++++++++++---- .../search/context/OGKUnifiedHighlighter.java | 61 ++--- .../opengrok/indexer/web/QueryParameters.java | 10 + .../opengrok/indexer/web/SearchHelper.java | 15 ++ .../indexer/analysis/JFlexXrefTest.java | 28 +-- .../indexer/analysis/XrefTestBase.java | 2 +- .../analysis/haskell/HaskellXrefTest.java | 14 +- .../indexer/analysis/php/PhpXrefTest.java | 29 ++- .../indexer/analysis/ruby/RubyXrefTest.java | 2 +- .../search/context/ContextFormatterTest.java | 209 +++++++++++------- .../SearchAndContextFormatterTest.java | 26 ++- .../SearchAndContextFormatterTest2.java | 38 ++-- .../indexer/util/CustomAssertions.java | 16 +- .../indexer/web/SearchHelperTest.java | 4 +- .../java/org/opengrok/web/PageConfig.java | 59 ++++- .../{print-1.0.2.css => print-1.1.0.css} | 62 +++++- .../{style-1.0.4.css => style-1.1.0.css} | 34 ++- opengrok-web/src/main/webapp/httpheader.jspf | 4 +- opengrok-web/src/main/webapp/menu.jspf | 38 ++-- opengrok-web/src/main/webapp/minisearch.jspf | 18 +- opengrok-web/src/main/webapp/more.jsp | 10 +- opengrok-web/src/main/webapp/search.jsp | 11 +- 25 files changed, 616 insertions(+), 301 deletions(-) rename opengrok-web/src/main/webapp/default/{print-1.0.2.css => print-1.1.0.css} (91%) rename opengrok-web/src/main/webapp/default/{style-1.0.4.css => style-1.1.0.css} (96%) diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/search/Results.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/search/Results.java index 9f59868cb42..08a020d9769 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/search/Results.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/search/Results.java @@ -141,13 +141,13 @@ private static Reader getXrefReader( /** * Prints out results in html form. The following search helper fields are * required to be properly initialized: * * @param out write destination @@ -271,9 +271,9 @@ private static void printPlain(PrintPlainFinalArgs fargs, Document doc, fargs.shelp.getSourceContext().toggleAlt(); - boolean didPresentNew = fargs.shelp.getSourceContext().getContext2(fargs.env, + boolean didPresentNew = fargs.shelp.getSourceContext().getContext2( fargs.shelp.getSearcher(), docId, fargs.out, fargs.xrefPrefix, - fargs.morePrefix, true, fargs.tabSize); + fargs.morePrefix, fargs.shelp.getLimitedContextArgs(), fargs.tabSize); if (!didPresentNew) { /* diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/search/context/Context.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/search/context/Context.java index 57cbaf8f433..aaa230729ba 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/search/context/Context.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/search/context/Context.java @@ -47,6 +47,7 @@ import org.opengrok.indexer.search.Hit; import org.opengrok.indexer.search.QueryBuilder; import org.opengrok.indexer.util.IOUtils; +import org.opengrok.indexer.web.QueryParameters; import org.opengrok.indexer.web.Util; /** @@ -112,26 +113,19 @@ public boolean isEmpty() { /** * Look for context for this instance's initialized query in a search result * {@link Document}, and output according to the parameters. - * @param env required environment * @param searcher required search that produced the document * @param docId document ID for producing context * @param dest required target to write * @param urlPrefix prefix for links * @param morePrefix optional link to more... page - * @param limit a value indicating if the number of matching lines should be - * limited. N.b. unlike - * {@link #getContext(java.io.Reader, java.io.Writer, java.lang.String, java.lang.String, java.lang.String, - * org.opengrok.indexer.analysis.Definitions, boolean, boolean, java.util.List, org.opengrok.indexer.analysis.Scopes)}, - * the {@code limit} argument will not be interpreted w.r.t. - * {@link RuntimeEnvironment#isQuickContextScan()}. + * @param contextArgs a value specifying how much context to show * @param tabSize optional positive tab size that must accord with the value * used when indexing or else postings may be wrongly shifted until * re-indexing * @return Did it get any matching context? */ - public boolean getContext2(RuntimeEnvironment env, IndexSearcher searcher, - int docId, Appendable dest, String urlPrefix, String morePrefix, - boolean limit, int tabSize) { + public boolean getContext2(IndexSearcher searcher, int docId, Appendable dest, + String urlPrefix, String morePrefix, ContextArgs contextArgs, int tabSize) { if (isEmpty()) { return false; @@ -178,35 +172,29 @@ public boolean getContext2(RuntimeEnvironment env, IndexSearcher searcher, String path = doc.get(QueryBuilder.PATH); String pathE = Util.uriEncodePath(path); - String urlPrefixE = urlPrefix == null ? "" : Util.uriEncodePath(urlPrefix); - String moreURL = morePrefix == null ? null : Util.uriEncodePath(morePrefix) + pathE + "?" + queryAsURI; + String urlPrefixE = urlPrefix == null ? "" : Util.uriEncodePath( + urlPrefix); + String moreURL = morePrefix == null ? null : Util.uriEncodePath(morePrefix) + pathE + "?" + + QueryParameters.CONTEXT_SURROUND_PARAM_EQ + contextArgs.getContextSurround() + + "&" + queryAsURI; - ContextArgs args = new ContextArgs(env.getContextSurround(), env.getContextLimit()); - /* - * Lucene adds to the following value in FieldHighlighter, so avoid - * integer overflow by not using Integer.MAX_VALUE -- Short is good - * enough. - */ - int linelimit = limit ? args.getContextLimit() : Short.MAX_VALUE; - - ContextFormatter formatter = new ContextFormatter(args); + ContextFormatter formatter = new ContextFormatter(contextArgs); formatter.setUrl(urlPrefixE + pathE); formatter.setDefs(tags); formatter.setScopes(scopes); formatter.setMoreUrl(moreURL); - formatter.setMoreLimit(linelimit); - OGKUnifiedHighlighter uhi = new OGKUnifiedHighlighter(env, searcher, anz); + OGKUnifiedHighlighter uhi = new OGKUnifiedHighlighter(searcher, anz); uhi.setBreakIterator(StrictLineBreakIterator::new); uhi.setFormatter(formatter); uhi.setTabSize(tabSize); + uhi.setContextArgs(contextArgs); try { List fieldList = qbuilder.getContextFields(); String[] fields = fieldList.toArray(new String[0]); - String res = uhi.highlightFieldsUnion(fields, query, docId, - linelimit); + String res = uhi.highlightFieldsUnion(fields, query, docId); if (res != null) { dest.append(res); return true; diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/search/context/ContextArgs.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/search/context/ContextArgs.java index 227fd67252b..70d5f900015 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/search/context/ContextArgs.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/search/context/ContextArgs.java @@ -27,6 +27,13 @@ * to producing context presentations. */ public class ContextArgs { + /** + * The maximum number of lines of surrounding context in a single direction + * to allow during a search. If there is sufficient content both preceding + * and following a hit, then the total context might be twice this value. + */ + public static final short MAX_CONTEXT_SURROUND = 4; + /** Not Lucene-related, so {@code int} does fine. */ private static final int CONTEXT_WIDTH = 100; diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/search/context/ContextFormatter.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/search/context/ContextFormatter.java index e646d2ceb52..b7577232cba 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/search/context/ContextFormatter.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/search/context/ContextFormatter.java @@ -24,7 +24,9 @@ import java.io.IOException; import java.util.ArrayList; +import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.SortedMap; import java.util.logging.Level; import java.util.logging.Logger; @@ -59,16 +61,19 @@ public class ContextFormatter extends PassageFormatter { private final PassageConverter cvt; private final List marks = new ArrayList<>(); + private final ContextArgs contextArgs; + private final HashSet reportedScopes = new HashSet<>(); private String url; private Definitions defs; private Scopes scopes; + private boolean showingContext; + private boolean showOverline; /** - * An optional URL for linking when the {@link #moreLimit} (if positive) is + * An optional URL for linking when the "moreLimit" (if positive) is * reached. */ private String moreUrl; - private int moreLimit; /** * Cached splitter, keyed by {@link #originalText}. @@ -82,6 +87,7 @@ public class ContextFormatter extends PassageFormatter { */ public ContextFormatter(ContextArgs args) { this.cvt = new PassageConverter(args); + this.contextArgs = args; } /** @@ -109,7 +115,7 @@ public void setUrl(String value) { } /** - * Gets the optional URL to use if {@link #getMoreLimit()} is reached. + * Gets the optional URL to use if the "more" limit is reached. * @return the URL or {@code null} */ public String getMoreUrl() { @@ -117,36 +123,13 @@ public String getMoreUrl() { } /** - * Sets the optional URL to use if {@link #getMoreLimit()} is reached. + * Sets the optional URL to use if the "more" limit is reached. * @param value the URL to use */ public void setMoreUrl(String value) { this.moreUrl = value; } - /** - * Gets the optional line limit to specify (if positive) a maximum number - * of lines to format and -- if {@link #getMoreUrl()} is defined -- a "more" - * link to display. Default is zero (i.e. inactive). - * @return the line limit value - */ - public int getMoreLimit() { - return moreLimit; - } - - /** - * Sets the optional line limit to specify (if positive) a maximum number - * of lines to format and -- if {@link #getMoreUrl()} is defined -- a "more" - * link to display. - * @param value the line limit - */ - public void setMoreLimit(int value) { - if (value < 0) { - throw new IllegalArgumentException("value is negative"); - } - this.moreLimit = value; - } - /** * Gets the optional definitions. * @return the defs @@ -205,23 +188,36 @@ public Object format(Passage[] passages, String originalText) { FormattedLines res = new FormattedLines(); StringBuilder bld = new StringBuilder(); - SortedMap lines = cvt.convert(passages, - splitter); + SortedMap lines = cvt.convert(passages, splitter); int numl = 0; + int contextLimit = calculateContextLimit(lines); boolean limited = false; - for (LineHighlight lhi : lines.values()) { - ++numl; - if (moreLimit > 0 && numl > moreLimit) { + int lastLineno = lines.isEmpty() ? 0 : lines.firstKey(); + for (Map.Entry entry : lines.entrySet()) { + if (++numl > contextLimit) { limited = true; break; } + LineHighlight lhi = entry.getValue(); String line = splitter.getLine(lhi.getLineno()); Matcher eolMatcher = StringUtils.STANDARD_EOL.matcher(line); if (eolMatcher.find()) { line = line.substring(0, eolMatcher.start()); } + /* + * When showing context, determine if the current line is non- + * consecutive in order to show an overline. + */ + showingContext = cvt.getArgs().getContextSurround() > 0; + if (!showingContext) { + showOverline = false; + } else { + showOverline = lhi.getLineno() - lastLineno > 1; + lastLineno = lhi.getLineno(); + } + try { marks.clear(); startLine(bld, lineUrl, lhi.getLineno()); @@ -268,10 +264,8 @@ public Object format(Passage[] passages, String originalText) { bld.append(HtmlConsts.ZB); } - finishLine(bld, lhi.getLineno(), marks); - // Regardless of true EOL, write a
. - bld.append(HtmlConsts.BR); - /** + finishLine(bld, lhi.getLineno(), lhi.countMarkups() > 0); + /* * Appending a LF here would hurt the more.jsp view, while * search.jsp (where getContext() does it) is indifferent -- so * skip it. @@ -299,8 +293,15 @@ public Object format(Passage[] passages, String originalText) { return res; } - private void startLine(Appendable dest, String lineUrl, int lineOffset) - throws IOException { + private void startLine(Appendable dest, String lineUrl, int lineOffset) throws IOException { + if (showingContext) { + if (showOverline) { + dest.append(""); + reportedScopes.clear(); + } else { + dest.append(""); + } + } dest.append(" "); } - private void finishLine(Appendable dest, int lineOffset, List marks) + private void finishLine(Appendable dest, int lineOffset, boolean hasHighlights) throws IOException { dest.append(""); - writeScope(lineOffset, dest); - writeTag(lineOffset, dest, marks); + if (hasHighlights) { + writeTag(lineOffset, dest); + writeScope(lineOffset, dest); + } + + // Regardless of true EOL, write a
. + dest.append(HtmlConsts.BR); + if (showingContext) { + dest.append(HtmlConsts.ZSPAN); + } } - private void writeScope(int lineOffset, Appendable dest) - throws IOException { + private void writeScope(int lineOffset, Appendable dest) throws IOException { Scopes.Scope scope = null; if (scopes != null) { // N.b. use ctags 1-based indexing vs 0-based. scope = scopes.getScope(lineOffset + 1); } - if (scope != null && scope != scopes.getScope(-1)) { + if (scope != null && isOkToReportScope(scope)) { dest.append(" marks) - throws IOException { + private boolean isOkToReportScope(Scopes.Scope scope) { + if (scope != scopes.getScope(-1)) { + if (!showingContext) { + return true; + } + if (!reportedScopes.contains(scope.getLineFrom())) { + reportedScopes.add(scope.getLineFrom()); + return true; + } + } + return false; + } + + private void writeTag(int lineOffset, Appendable dest) throws IOException { if (defs != null) { // N.b. use ctags 1-based indexing vs 0-based. List linetags = defs.getTags(lineOffset + 1); if (linetags != null) { - Tag pickedTag = findTagForMark(linetags, marks); + Tag pickedTag = findTagForMark(linetags); if (pickedTag != null) { dest.append(" "); Util.htmlize(pickedTag.type, dest); @@ -358,7 +378,7 @@ private void writeTag(int lineOffset, Appendable dest, List marks) * character is a non-word ({@code (?U)\W}) character. * @return a defined instance or {@code null} */ - private Tag findTagForMark(List linetags, List marks) { + private Tag findTagForMark(List linetags) { for (Tag tag : linetags) { if (tag.type != null) { for (String mark : marks) { @@ -377,4 +397,52 @@ private static boolean isNonWord(char c) { String cword = String.valueOf(c); return NONWORD_CHAR.matcher(cword).matches(); } + + /** + * Calculates a context limit when surrounding context is enabled to + * possibly reduce below moreLimit so that dangling surrounding context is + * not displayed. + */ + private int calculateContextLimit(SortedMap lines) { + int moreLimit = contextArgs.getContextLimit(); + // If moreLimit is not applicable, then leave unbounded. + if (moreLimit < 1) { + return Integer.MAX_VALUE; + } + /* + * If no surrounding context is specified or if the lines are already + * within moreLimit, then just use moreLimit. + */ + if (cvt.getArgs().getContextSurround() < 1 || lines.size() <= moreLimit) { + return moreLimit; + } + /* + * Walk back from moreLimit to ensure not to leave any dangling + * surrounding context by making sure there is a phrase highlight seen + * before a non-contiguous lineno. + */ + LineHighlight[] lineHighlights = lines.values().toArray(new LineHighlight[0]); + int i = moreLimit - 1; + int lastLineno = lineHighlights[i].getLineno(); + int newMoreLimit = moreLimit; + int trailingCount = 0; + for (; i >= 0; --i) { + LineHighlight lhi = lineHighlights[i]; + if (lhi.countMarkups() > 0) { + break; + } + if (lastLineno - lhi.getLineno() > 1) { + // Found non-contiguity without having seen a highlight. + newMoreLimit = i + 1; + trailingCount = 1; + } else if (trailingCount + 1 > contextArgs.getContextSurround()) { + // Found superfluous surrounding context. + --newMoreLimit; + } else { + ++trailingCount; + } + lastLineno = lhi.getLineno(); + } + return newMoreLimit; + } } diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/search/context/OGKUnifiedHighlighter.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/search/context/OGKUnifiedHighlighter.java index 65c32efb45e..20d501fc18b 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/search/context/OGKUnifiedHighlighter.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/search/context/OGKUnifiedHighlighter.java @@ -37,6 +37,7 @@ import org.apache.lucene.search.DocIdSetIterator; import org.apache.lucene.search.IndexSearcher; import org.apache.lucene.search.Query; +import org.apache.lucene.search.uhighlight.PassageFormatter; import org.apache.lucene.search.uhighlight.UHComponents; import org.apache.lucene.search.uhighlight.UnifiedHighlighter; import org.apache.lucene.util.BytesRef; @@ -58,35 +59,30 @@ public class OGKUnifiedHighlighter extends UnifiedHighlighter { private static final Logger LOGGER = LoggerFactory.getLogger( OGKUnifiedHighlighter.class); - private final RuntimeEnvironment env; + private final RuntimeEnvironment env = RuntimeEnvironment.getInstance(); private int tabSize; private String fileTypeName; + private ContextArgs contextArgs; + /** * Initializes an instance with * {@link UnifiedHighlighter#UnifiedHighlighter(org.apache.lucene.search.IndexSearcher, org.apache.lucene.analysis.Analyzer)} * for the specified {@code indexSearcher} and {@code indexAnalyzer}, and * stores the {@code env} for later use. - * @param env a required instance * @param indexSearcher a required instance * @param indexAnalyzer a required instance * @throws IllegalArgumentException if any argument is null */ - public OGKUnifiedHighlighter(RuntimeEnvironment env, - IndexSearcher indexSearcher, Analyzer indexAnalyzer) { + public OGKUnifiedHighlighter(IndexSearcher indexSearcher, Analyzer indexAnalyzer) { super(indexSearcher, indexAnalyzer); - - if (env == null) { - throw new IllegalArgumentException("env is null"); - } - this.env = env; } /** * Gets a file type name-specific analyzer during the execution of - * {@link #highlightFieldsUnion(java.lang.String[], org.apache.lucene.search.Query, int, int)}, + * {@link #highlightFieldsUnion(java.lang.String[], org.apache.lucene.search.Query, int)}, * or just gets the object passed in to the constructor at all other times. * @return a defined instance */ @@ -108,20 +104,28 @@ public void setTabSize(int value) { this.tabSize = value; } + /** + * Sets the instance used (if applicable) for the {@link PassageFormatter} + * assigned to {@link #setFormatter(PassageFormatter)}. + * @param contextArgs a defined instance or {@code null} + */ + public void setContextArgs(ContextArgs contextArgs) { + this.contextArgs = contextArgs; + } + /** * Transiently arranges that {@link #getIndexAnalyzer()} returns a file type * name-specific analyzer during a subsequent call of - * {@link #highlightFieldsUnionWork(java.lang.String[], org.apache.lucene.search.Query, int, int)}. + * {@link #highlightFieldsUnionWork(java.lang.String[], org.apache.lucene.search.Query, int)}. * @param fields a defined instance * @param query a defined instance * @param docId a valid document ID - * @param lineLimit the maximum number of lines to return * @return a defined instance or else {@code null} if there are no results * @throws IOException if accessing the Lucene document fails */ - public String highlightFieldsUnion(String[] fields, Query query, - int docId, int lineLimit) throws IOException { - /** + public String highlightFieldsUnion(String[] fields, Query query, int docId) + throws IOException { + /* * Setting fileTypeName has to happen before getFieldHighlighter() is * called by highlightFieldsAsObjects() so that the result of * getIndexAnalyzer() (if it is called due to requiring ANALYSIS) can be @@ -130,7 +134,7 @@ public String highlightFieldsUnion(String[] fields, Query query, Document doc = searcher.doc(docId); fileTypeName = doc == null ? null : doc.get(QueryBuilder.TYPE); try { - return highlightFieldsUnionWork(fields, query, docId, lineLimit); + return highlightFieldsUnionWork(fields, query, docId); } finally { fileTypeName = null; } @@ -144,25 +148,30 @@ public String highlightFieldsUnion(String[] fields, Query query, * @param fields a defined instance * @param query a defined instance * @param docId a valid document ID - * @param lineLimit the maximum number of lines to return * @return a defined instance or else {@code null} if there are no results * @throws IOException if accessing the Lucene document fails */ - protected String highlightFieldsUnionWork(String[] fields, Query query, - int docId, int lineLimit) throws IOException { + protected String highlightFieldsUnionWork(String[] fields, Query query, int docId) + throws IOException { + + int lineLimit = contextArgs != null ? contextArgs.getContextLimit() : Short.MAX_VALUE; int[] maxPassagesCopy = new int[fields.length]; - /** + /* * N.b. linelimit + 1 so that the ContextFormatter has an indication - * when to display the "more..." link. + * when to display the "more..." link. If we're showing surrounding + * context, though, then leave this essentially unbounded (initially), + * or else Lucene's BM25 sampling when limited can result in highlights + * confusingly missing from surrounding context. */ - Arrays.fill(maxPassagesCopy, lineLimit + 1); + Arrays.fill(maxPassagesCopy, contextArgs != null && + contextArgs.getContextSurround() > 0 ? Short.MAX_VALUE : lineLimit + 1); FormattedLines res = null; Map mappedRes = highlightFieldsAsObjects(fields, query, new int[]{docId}, maxPassagesCopy); - for (Object[] flinesz : mappedRes.values()) { - for (Object obj : flinesz) { - /** + for (Object[] passageObjects : mappedRes.values()) { + for (Object obj : passageObjects) { + /* * Empirical testing showed that the passage could be null if * the original source text is not available to the highlighter. */ @@ -246,7 +255,7 @@ protected OffsetSource getOptimizedOffsetSource(UHComponents components) { OffsetSource res = super.getOptimizedOffsetSource(components); String field = components.getField(); if (res == OffsetSource.ANALYSIS) { - /** + /* * Testing showed that UnifiedHighlighter falls back to * ANALYSIS in the presence of multi-term queries (MTQs) such as * prefixes and wildcards even for fields that are analyzed with diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/web/QueryParameters.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/web/QueryParameters.java index 2af5e3dacdf..b26ce57867a 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/web/QueryParameters.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/web/QueryParameters.java @@ -36,6 +36,16 @@ public class QueryParameters { */ public static final String ANNOTATION_PARAM_EQ_TRUE = ANNOTATION_PARAM + "=true"; + /** + * Parameter name to specify a number of context lines. + */ + public static final String CONTEXT_SURROUND_PARAM = "x"; + + /** + * {@link #CONTEXT_SURROUND_PARAM} concatenated with {@code "=" }. + */ + public static final String CONTEXT_SURROUND_PARAM_EQ = CONTEXT_SURROUND_PARAM + "="; + /** * Parameter name to specify a count value. */ diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/web/SearchHelper.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/web/SearchHelper.java index b7619f0cd11..a03eb8197d9 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/web/SearchHelper.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/web/SearchHelper.java @@ -78,6 +78,7 @@ import org.opengrok.indexer.search.SettingsHelper; import org.opengrok.indexer.search.Summarizer; import org.opengrok.indexer.search.context.Context; +import org.opengrok.indexer.search.context.ContextArgs; import org.opengrok.indexer.search.context.HistoryContext; import org.opengrok.indexer.util.ForbiddenSymlinkException; import org.opengrok.indexer.util.IOUtils; @@ -133,6 +134,10 @@ public class SearchHelper { * max. number of result items to show */ private final int maxItems; + /** + * args to use when context is limited. + */ + private final ContextArgs limitedContextArgs; /** * The QueryBuilder used to create the query. */ @@ -235,6 +240,7 @@ public class SearchHelper { public SearchHelper(int start, SortOrder sortOrder, File dataRoot, File sourceRoot, int maxItems, EftarFileReader eftarFileReader, QueryBuilder queryBuilder, boolean crossRefSearch, + ContextArgs limitedContextArgs, String contextPath, boolean guiSearch, boolean noRedirect) { this.start = start; this.order = sortOrder; @@ -244,6 +250,7 @@ public SearchHelper(int start, SortOrder sortOrder, File dataRoot, File sourceRo this.desc = eftarFileReader; this.builder = queryBuilder; this.crossRefSearch = crossRefSearch; + this.limitedContextArgs = limitedContextArgs; this.contextPath = contextPath; this.guiSearch = guiSearch; this.noRedirect = noRedirect; @@ -265,6 +272,10 @@ public QueryBuilder getBuilder() { return builder; } + public ContextArgs getLimitedContextArgs() { + return limitedContextArgs; + } + public String getContextPath() { return contextPath; } @@ -518,6 +529,10 @@ public SearchHelper executeQuery() { return this; } + public ContextArgs getUnlimitedContextArgs() { + return new ContextArgs(limitedContextArgs.getContextSurround(), Short.MAX_VALUE); + } + private void maybeRedirectToDefinition(int docID, TermQuery termQuery) throws IOException, ClassNotFoundException { // Bug #3900: Check if this is a search for a single term, and that diff --git a/opengrok-indexer/src/test/java/org/opengrok/indexer/analysis/JFlexXrefTest.java b/opengrok-indexer/src/test/java/org/opengrok/indexer/analysis/JFlexXrefTest.java index 6e0694621ea..adeb18b6f59 100644 --- a/opengrok-indexer/src/test/java/org/opengrok/indexer/analysis/JFlexXrefTest.java +++ b/opengrok-indexer/src/test/java/org/opengrok/indexer/analysis/JFlexXrefTest.java @@ -230,8 +230,8 @@ private void assertXrefLine(Class xrefClass, StringWriter output = new StringWriter(); xref.write(output); - assertLinesEqual("xref " + xrefClass.getSimpleName(), - FIRST_LINE_PREAMBLE + expectedOutput, output.toString()); + assertLinesEqual(FIRST_LINE_PREAMBLE + expectedOutput, output.toString(), "xref " + + xrefClass.getSimpleName()); } /** @@ -248,9 +248,8 @@ void bug16883() throws Exception { new StringReader(ECHO_QUOT_XYZ))); StringWriter out = new StringWriter(); xref.write(out); - assertLinesEqual("Unterminated string:\n" + ECHO_QUOT_XYZ, - FIRST_LINE_PREAMBLE + - "echo "xyz", out.toString()); + assertLinesEqual(FIRST_LINE_PREAMBLE + "echo "xyz", + out.toString(), "Unterminated string:\n" + ECHO_QUOT_XYZ); // Reuse the xref and verify that the broken syntax in the previous // file doesn't cause broken highlighting in the next file @@ -259,10 +258,8 @@ void bug16883() throws Exception { xref.setReader(new StringReader(new String(contents.toCharArray()))); xref.reset(); xref.write(out); - assertLinesEqual("reused ShXref after broken syntax", - FIRST_LINE_PREAMBLE + - "echo "hello"", - out.toString()); + assertLinesEqual(FIRST_LINE_PREAMBLE + "echo "hello"", + out.toString(), "reused ShXref after broken syntax"); } /** @@ -413,13 +410,12 @@ void truncatedUuencodedFile() throws IOException { StringWriter out = new StringWriter(); xref.write(out); - assertLinesEqual("UuencodeXref truncated", - "1" - + "begin 644 " - + "test.txt" - + "\n" - + "2", - out.toString()); + assertLinesEqual("1" + + "begin 644 " + + "test.txt" + + "\n" + + "2", out.toString(), + "UuencodeXref truncated"); } /** diff --git a/opengrok-indexer/src/test/java/org/opengrok/indexer/analysis/XrefTestBase.java b/opengrok-indexer/src/test/java/org/opengrok/indexer/analysis/XrefTestBase.java index 06b0008af4b..fce803ed210 100644 --- a/opengrok-indexer/src/test/java/org/opengrok/indexer/analysis/XrefTestBase.java +++ b/opengrok-indexer/src/test/java/org/opengrok/indexer/analysis/XrefTestBase.java @@ -86,7 +86,7 @@ protected void writeAndCompare( String[] expected = expStr.split("\n"); String messagePrefix = factory.getClass().getName(); - assertLinesEqual(messagePrefix + " xref", expected, gotten); + assertLinesEqual(expected, gotten, messagePrefix + " xref"); assertEquals(expectedLOC, actLOC, messagePrefix + " LOC"); } diff --git a/opengrok-indexer/src/test/java/org/opengrok/indexer/analysis/haskell/HaskellXrefTest.java b/opengrok-indexer/src/test/java/org/opengrok/indexer/analysis/haskell/HaskellXrefTest.java index ea4b892166c..1e9ea57c158 100644 --- a/opengrok-indexer/src/test/java/org/opengrok/indexer/analysis/haskell/HaskellXrefTest.java +++ b/opengrok-indexer/src/test/java/org/opengrok/indexer/analysis/haskell/HaskellXrefTest.java @@ -61,12 +61,10 @@ public void basicTest() throws IOException { AbstractAnalyzer analyzer = fac.getAnalyzer(); WriteXrefArgs xargs = new WriteXrefArgs(new StringReader(s), w); Xrefer xref = analyzer.writeXref(xargs); - assertLinesEqual("Haskell basicTest", - "1" + - "putStrLn" + - " "Hello, world!"\n", - w.toString()); + assertLinesEqual("1" + + "putStrLn" + + " "Hello, world!"\n", w.toString(), "Haskell basicTest"); assertEquals(1, xref.getLOC(), "Haskell LOC"); } @@ -125,7 +123,7 @@ public void sampleTest() throws IOException { String[] actual = new String(sampleOutputStream.toByteArray(), StandardCharsets.UTF_8).split("\\r?\\n"); String[] expected = new String(expectedOutputSteam.toByteArray(), StandardCharsets.UTF_8).split("\\r?\\n"); - assertLinesEqual("Haskell sampleTest()", expected, actual); + assertLinesEqual(expected, actual, "Haskell sampleTest()"); assertEquals(3, actLOC, "Haskell LOC"); } @@ -167,7 +165,7 @@ private void writeAndCompare(String sourceResource, String resultResource, String estr = new String(expbytes, StandardCharsets.UTF_8); String[] expected = estr.split("\n"); - assertLinesEqual("Haskell xref", expected, gotten); + assertLinesEqual(expected, gotten, "Haskell xref"); assertEquals(expLOC, actLOC, "Haskell LOC"); } diff --git a/opengrok-indexer/src/test/java/org/opengrok/indexer/analysis/php/PhpXrefTest.java b/opengrok-indexer/src/test/java/org/opengrok/indexer/analysis/php/PhpXrefTest.java index e5539762b8d..62ecf30f894 100644 --- a/opengrok-indexer/src/test/java/org/opengrok/indexer/analysis/php/PhpXrefTest.java +++ b/opengrok-indexer/src/test/java/org/opengrok/indexer/analysis/php/PhpXrefTest.java @@ -78,20 +78,19 @@ public void basicSingleQuotedStringTest() throws IOException { AbstractAnalyzer analyzer = fac.getAnalyzer(); WriteXrefArgs xargs = new WriteXrefArgs(new StringReader(s), w); Xrefer xref = analyzer.writeXref(xargs); - assertLinesEqual("PHP quoting", - "1<?php " - + "define("FOO"," - + " 'BAR\\'"'); " - + "$foo='bar'; " - + "$hola="ls"; " - + "$hola=''; " - + "$hola="";", - w.toString()); + assertLinesEqual("1<?php " + + "define("FOO", " + + "'BAR\\'"'); " + + "$foo='bar'; " + + "$hola="ls"; " + + "$hola=''; " + + "$hola="";", + w.toString(), "PHP quoting"); assertEquals(1, xref.getLOC(), "PHP LOC"); } @@ -146,7 +145,7 @@ public void sampleTest() throws IOException { String[] gotten = new String(baos.toByteArray(), StandardCharsets.UTF_8).split("\\r?\\n"); String[] expected = new String(expbytes, StandardCharsets.UTF_8).split("\n"); - assertLinesEqual("PHP xref", expected, gotten); + assertLinesEqual(expected, gotten, "PHP xref"); assertEquals(29, actLOC, "PHP LOC"); assertEquals(expected.length, gotten.length); diff --git a/opengrok-indexer/src/test/java/org/opengrok/indexer/analysis/ruby/RubyXrefTest.java b/opengrok-indexer/src/test/java/org/opengrok/indexer/analysis/ruby/RubyXrefTest.java index ef4969e3212..6c9f0db6200 100644 --- a/opengrok-indexer/src/test/java/org/opengrok/indexer/analysis/ruby/RubyXrefTest.java +++ b/opengrok-indexer/src/test/java/org/opengrok/indexer/analysis/ruby/RubyXrefTest.java @@ -66,7 +66,7 @@ public void colonQuoteAfterInterpolation() throws IOException { + "data-definition-place=\"undefined-in-file\">logfn" + "}:"\n" + "2\n"; - assertLinesEqual("Ruby colon-quote", xexpected, xout); + assertLinesEqual(xexpected, xout, "Ruby colon-quote"); assertEquals(1, actLOC, "Ruby colon-quote LOC"); } } diff --git a/opengrok-indexer/src/test/java/org/opengrok/indexer/search/context/ContextFormatterTest.java b/opengrok-indexer/src/test/java/org/opengrok/indexer/search/context/ContextFormatterTest.java index 7fa9f477199..7c9339832c4 100644 --- a/opengrok-indexer/src/test/java/org/opengrok/indexer/search/context/ContextFormatterTest.java +++ b/opengrok-indexer/src/test/java/org/opengrok/indexer/search/context/ContextFormatterTest.java @@ -47,7 +47,10 @@ public class ContextFormatterTest { " Aenean dignissim ipsum eu rhoncus ultricies.\n" + "\n" + " Fusce pretium hendrerit dictum. Pellentesque habitant\n" + - "morbi tristique senectus et netus."; + "morbi tristique senectus et netus.\n" + + "Nam scelerisque odio at justo fringilla, eu aliquet sem commodo.\n" + + "Duis aliquet non magna ac gravida. Aliquam erat volutpat. Proin\n" + + "nec iaculis mauris."; private static final String DOC2 = "abc\n" + "def\n" + @@ -74,10 +77,10 @@ public void testLineMatchFormatted() { final String DOCCTX_0 = "" + - "3 Mauris diam nisl, tincidunt nec gravida sit" + - " amet, efficitur vitae
\n"; + "3 Mauris diam nisl, tincidunt nec gravida sit " + + "amet, efficitur vitae
\n"; String ctx = res.toString(); - assertLinesEqual("format().toString()", DOCCTX_0, ctx); + assertLinesEqual(DOCCTX_0, ctx, "format().toString()"); // Second, test with contextCount==1 args = new ContextArgs((short) 1, (short) 10); @@ -86,16 +89,17 @@ public void testLineMatchFormatted() { res = fmt.format(new Passage[] {p}, DOC); assertNotNull(res, "format() result"); - final String DOCCTX_1 = "" + + final String DOCCTX_1 = + "" + "2 Mauris vel tortor vel nisl efficitur fermentum nec vel" + - " erat.
" + - "" + + " erat.
" + + "" + "3 Mauris diam nisl, tincidunt nec gravida sit" + - " amet, efficitur vitae
" + - "" + - "4 est. Sed aliquam non mi vel mattis:
"; + " amet, efficitur vitae
" + + "" + + "4 est. Sed aliquam non mi vel mattis:
"; ctx = res.toString(); - assertLinesEqual("format().toString()", DOCCTX_1, ctx); + assertLinesEqual(DOCCTX_1, ctx, "format().toString()"); } @Test @@ -106,7 +110,7 @@ public void testLinesSpanningMatchFormatted() { p.addMatch(0, p.getEndOffset(), new BytesRef(DOC2), 1); assertEquals(1, p.getNumMatches(), "getNumMatches()"); - /** + /* * We're using the entire document, but see how it behaves with * contextCount==1 */ @@ -117,14 +121,14 @@ public void testLinesSpanningMatchFormatted() { assertNotNull(res, "format() result"); final String DOC2CTX = - "" + - "1 abc
" + - "" + - "2 def
" + - "" + - "3 ghi
"; + "" + + "1 abc
" + + "" + + "2 def
" + + "" + + "3 ghi
"; String ctx = res.toString(); - assertLinesEqual("format().toString()", DOC2CTX, ctx); + assertLinesEqual(DOC2CTX, ctx, "format().toString()"); } @Test @@ -151,7 +155,7 @@ public void testDualElidedMatchFormatted() { " non ornare egestas. Aenean dignissim ipsum eu" + " rhoncus…
\n"; String ctx = res.toString(); - assertLinesEqual("format().toString()", DOCCTX_0, ctx); + assertLinesEqual(DOCCTX_0, ctx, "format().toString()"); // Second, test with contextCount==1 args = new ContextArgs((short) 1, (short) 10); @@ -160,35 +164,36 @@ public void testDualElidedMatchFormatted() { res = fmt.format(new Passage[] {p}, DOC); assertNotNull(res, "format() result"); - final String DOCCTX_1 = "" + - "5
" + - "" + + final String DOCCTX_1 = + "" + + "5
" + + "" + "6 …putate ipsum sed laoreet. Nam maximus libero" + " non ornare egestas. Aenean dignissim ipsum eu" + - " rhoncus…
" + - "" + - "7
"; + " rhoncus…
" + + "" + + "7
"; ctx = res.toString(); - assertLinesEqual("format().toString()", DOCCTX_1, ctx); + assertLinesEqual(DOCCTX_1, ctx, "format().toString()"); // Third, test with contextCount==1 and a line limit - args = new ContextArgs((short) 1, (short) 10); + args = new ContextArgs((short) 1, (short) 2); fmt = new ContextFormatter(args); fmt.setUrl("http://example.com"); - fmt.setMoreLimit(2); fmt.setMoreUrl("http://example.com/more"); res = fmt.format(new Passage[] {p}, DOC); assertNotNull(res, "format() result"); - final String DOCCTX_2M = "" + - "5
" + - "" + + final String DOCCTX_2M = + "" + + "5
" + + "" + "6 …putate ipsum sed laoreet. Nam maximus libero" + " non ornare egestas. Aenean dignissim ipsum eu" + - " rhoncus…
" + + " rhoncus…
" + "[all …]
"; ctx = res.toString(); - assertLinesEqual("format().toString()", DOCCTX_2M, ctx); + assertLinesEqual(DOCCTX_2M, ctx, "format().toString()"); } @Test @@ -212,12 +217,12 @@ public void testLeftElidedMatchFormatted() { final String DOCCTX_0 = "6 …um sed laoreet. Nam " + - "maximus libero non ornare egestas. Aenean " + - "dignissim ipsum eu rhoncus ultricies." + - "
"; + "class=\"l\">6 …um sed laoreet. Nam " + + "maximus libero non ornare egestas. Aenean " + + "dignissim ipsum eu rhoncus ultricies." + + "
"; String ctx = res.toString(); - assertLinesEqual("format().toString()", DOCCTX_0, ctx); + assertLinesEqual(DOCCTX_0, ctx, "format().toString()"); // Second, test with contextCount==1 args = new ContextArgs((short) 1, (short) 10); @@ -227,37 +232,38 @@ public void testLeftElidedMatchFormatted() { assertNotNull(res, "format() result"); final String DOCCTX_1 = - "5
" + - "6 …um sed laoreet. Nam " + - "maximus libero non ornare egestas. Aenean " + - "dignissim ipsum eu rhoncus ultricies." + - "
" + - "7
"; + "5
" + + "6 …um sed laoreet. Nam " + + "maximus libero non ornare egestas. Aenean " + + "dignissim ipsum eu rhoncus ultricies." + + "
" + + "7
"; ctx = res.toString(); - assertLinesEqual("format().toString()", DOCCTX_1, ctx); + assertLinesEqual(DOCCTX_1, ctx, "format().toString()"); // Third, test with contextCount==1 and a line limit - args = new ContextArgs((short) 1, (short) 10); + args = new ContextArgs((short) 1, (short) 2); fmt = new ContextFormatter(args); fmt.setUrl("http://example.com"); - fmt.setMoreLimit(2); fmt.setMoreUrl("http://example.com/more"); res = fmt.format(new Passage[] {p}, DOC); assertNotNull(res, "format() result"); - final String DOCCTX_2M = "" + - "5
" + - "" + + "5
" + + "6 …um sed laoreet. Nam " + "maximus libero non ornare egestas. Aenean " + "dignissim ipsum eu rhoncus ultricies." + - "
[all " + + "
" + + "[all " + "…]
"; ctx = res.toString(); - assertLinesEqual("format().toString()", DOCCTX_2M, ctx); + assertLinesEqual(DOCCTX_2M, ctx, "format().toString()"); } @Test @@ -284,7 +290,7 @@ public void testRightElidedMatchFormatted() { "lacus velit varius vulputate ipsum sed laoreet. " + "Nam maximus libero non ornare eg…
"; String ctx = res.toString(); - assertLinesEqual("format().toString()", DOCCTX_0, ctx); + assertLinesEqual(DOCCTX_0, ctx, "format().toString()"); // Second, test with contextCount==1 args = new ContextArgs((short) 1, (short) 10); @@ -294,36 +300,36 @@ public void testRightElidedMatchFormatted() { assertNotNull(res, "format() result"); final String DOCCTX_1 = - "5
" + - "6 ----Maecenas vitae " + - "lacus velit varius vulputate ipsum sed laoreet. " + - "Nam maximus libero non ornare eg…
" + - "7
"; + "5
" + + "6 ----Maecenas vitae " + + "lacus velit varius vulputate ipsum sed laoreet. " + + "Nam maximus libero non ornare eg…
" + + "7
"; ctx = res.toString(); - assertLinesEqual("format().toString()", DOCCTX_1, ctx); + assertLinesEqual(DOCCTX_1, ctx, "format().toString()"); // Third, test with contextCount==1 and a line limit - args = new ContextArgs((short) 1, (short) 10); + args = new ContextArgs((short) 1, (short) 2); fmt = new ContextFormatter(args); fmt.setUrl("http://example.com"); - fmt.setMoreLimit(2); fmt.setMoreUrl("http://example.com/more"); res = fmt.format(new Passage[] {p}, DOC); assertNotNull(res, "format() result"); - final String DOCCTX_2M = "5
" + - "5
" + + "6 ----Maecenas vitae " + "lacus velit varius vulputate ipsum sed laoreet. " + - "Nam maximus libero non ornare eg…
" + + "Nam maximus libero non ornare eg…
" + "[all " + "…]
\n"; ctx = res.toString(); - assertLinesEqual("format().toString()", DOCCTX_2M, ctx); + assertLinesEqual(DOCCTX_2M, ctx, "format().toString()"); } @Test @@ -354,6 +360,61 @@ public void testBoundsProblemFormatted() { "3 Mauris diam nisl, tincidunt nec gravida sit" + " amet, efficitur vitae
\n"; String ctx = res.toString(); - assertLinesEqual("format().toString()", DOC_CTX_0, ctx); + assertLinesEqual(DOC_CTX_0, ctx, "format().toString()"); + } + + @Test + public void testRecalculatedContextLimit() { + final String PHRASE1 = "efficitur fermentum"; + final String PHRASE2 = "varius vulputate"; + final String PHRASE3 = "justo fringilla"; + + Passage passage1 = new Passage(); + int phraseOff1 = DOC.indexOf(PHRASE1); + passage1.setStartOffset(phraseOff1); + passage1.setEndOffset(phraseOff1 + PHRASE1.length()); + assertTrue(passage1.getStartOffset() >= 0 && passage1.getEndOffset() > + passage1.getStartOffset(), "passage1 offsets are positive"); + passage1.addMatch(passage1.getStartOffset(), passage1.getEndOffset(), new BytesRef(PHRASE1), 1); + + Passage passage2 = new Passage(); + int phraseOff2 = DOC.indexOf(PHRASE2); + passage2.setStartOffset(phraseOff2); + passage2.setEndOffset(phraseOff2 + PHRASE2.length()); + assertTrue(passage2.getStartOffset() >= 0 && passage2.getEndOffset() > + passage2.getStartOffset(), "passage2 offsets are positive"); + passage2.addMatch(passage2.getStartOffset(), passage2.getEndOffset(), new BytesRef(PHRASE2), 1); + + Passage passage3 = new Passage(); + int phraseOff3 = DOC.indexOf(PHRASE3); + passage3.setStartOffset(phraseOff3); + passage3.setEndOffset(phraseOff3 + PHRASE3.length()); + assertTrue(passage3.getStartOffset() >= 0 && passage3.getEndOffset() > + passage3.getStartOffset(), "passage3 offsets are positive"); + passage3.addMatch(passage3.getStartOffset(), passage3.getEndOffset(), new BytesRef(PHRASE3), 1); + + // Test with non-zero context limit. + ContextArgs args = new ContextArgs((short) 1, (short) 7); + ContextFormatter fmt = new ContextFormatter(args); + fmt.setUrl("http://example.com"); + Object formatted = fmt.format(new Passage[] {passage1, passage2, passage3}, DOC); + assertNotNull(formatted, "format() result"); + + final String DOC_CTX_0 = "" + + "1" + + " Lorem ipsum dolor sit amet, consectetur adipiscing elit.
" + + "" + + "2 Mauris vel tortor vel nisl " + + "efficitur fermentum nec vel erat.
" + + "3 " + + "Mauris diam nisl, tincidunt nec gravida sit amet, efficitur vitae
" + + "" + + "5
" + + "6 " + + "----Maecenas vitae lacus velit varius vulputate ipsum sed laoreet. " + + "Nam maximus libero non ornare eg…
" + + "" + + "7
"; + assertLinesEqual(DOC_CTX_0, formatted.toString(), "format().toString()"); } } diff --git a/opengrok-indexer/src/test/java/org/opengrok/indexer/search/context/SearchAndContextFormatterTest.java b/opengrok-indexer/src/test/java/org/opengrok/indexer/search/context/SearchAndContextFormatterTest.java index c25b915a85e..5a76a50fb9f 100644 --- a/opengrok-indexer/src/test/java/org/opengrok/indexer/search/context/SearchAndContextFormatterTest.java +++ b/opengrok-indexer/src/test/java/org/opengrok/indexer/search/context/SearchAndContextFormatterTest.java @@ -33,6 +33,7 @@ import org.apache.lucene.search.ScoreDoc; import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import org.opengrok.indexer.analysis.AbstractAnalyzer; @@ -61,13 +62,16 @@ class SearchAndContextFormatterTest { private static RuntimeEnvironment env; private static TestRepository repository; private static File configFile; + private static boolean savedHistoryEnabled; @BeforeAll public static void setUpClass() throws Exception { + env = RuntimeEnvironment.getInstance(); + savedHistoryEnabled = env.isHistoryEnabled(); + repository = new TestRepository(); repository.create(HistoryGuru.class.getResource("/repositories")); - env = RuntimeEnvironment.getInstance(); env.setCtags(System.getProperty("org.opengrok.indexer.analysis.Ctags", "ctags")); env.setSourceRoot(repository.getSourceRoot()); env.setDataRoot(repository.getDataRoot()); @@ -92,6 +96,11 @@ public static void tearDownClass() { configFile.delete(); } + @AfterEach + public void tearDown() { + env.setHistoryEnabled(savedHistoryEnabled); + } + @Test void testSearch() throws IOException { SearchEngine instance = new SearchEngine(); @@ -103,12 +112,13 @@ void testSearch() throws IOException { assertNotNull(frags, "getFirstFragments() should return something"); assertEquals(1, frags.length, "frags should have one element"); - final String CTX = "12" - + " std::random_device rd;
" - + "13 std::mt19937 gen(rd());
14 " - + "std::uniform_int_distribution<> dis(0, RAND_MAX);
"; - assertLinesEqual("ContextFormatter output", CTX, frags[0]); + final String CTX = "" + + "12 std::random_device rd;
" + + "13 " + + "std::mt19937 gen(rd());
" + + "14 " + + "std::uniform_int_distribution<> dis(0, RAND_MAX);
"; + assertLinesEqual(CTX, frags[0], "ContextFormatter output"); instance.destroy(); } @@ -124,7 +134,7 @@ private String[] getFirstFragments(SearchEngine instance) throws IOException { AbstractAnalyzer anz = fac.getAnalyzer(); ContextFormatter formatter = new ContextFormatter(args); - OGKUnifiedHighlighter uhi = new OGKUnifiedHighlighter(env, instance.getSearcher(), anz); + OGKUnifiedHighlighter uhi = new OGKUnifiedHighlighter(instance.getSearcher(), anz); uhi.setBreakIterator(StrictLineBreakIterator::new); uhi.setFormatter(formatter); diff --git a/opengrok-indexer/src/test/java/org/opengrok/indexer/search/context/SearchAndContextFormatterTest2.java b/opengrok-indexer/src/test/java/org/opengrok/indexer/search/context/SearchAndContextFormatterTest2.java index b503d34e890..afd9eece5ff 100644 --- a/opengrok-indexer/src/test/java/org/opengrok/indexer/search/context/SearchAndContextFormatterTest2.java +++ b/opengrok-indexer/src/test/java/org/opengrok/indexer/search/context/SearchAndContextFormatterTest2.java @@ -37,9 +37,9 @@ import org.apache.lucene.document.Document; import org.apache.lucene.search.ScoreDoc; -import org.apache.lucene.search.highlight.InvalidTokenOffsetsException; import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import org.opengrok.indexer.analysis.AbstractAnalyzer; @@ -75,13 +75,16 @@ public class SearchAndContextFormatterTest2 { private static TestRepository repository1; private static TestRepository repository2; private static File configFile; - private static boolean originalProjectsEnabled; + private static boolean savedProjectsEnabled; + private static Set savedAllowedSymlinks; @BeforeAll public static void setUpClass() throws Exception { env = RuntimeEnvironment.getInstance(); - originalProjectsEnabled = env.isProjectsEnabled(); + savedProjectsEnabled = env.isProjectsEnabled(); + savedAllowedSymlinks = new HashSet<>(env.getAllowedSymlinks()); + env.setProjectsEnabled(true); File sourceRoot = createTemporaryDirectory("srcroot"); @@ -137,9 +140,6 @@ public static void setUpClass() throws Exception { @AfterAll public static void tearDownClass() { - env.setProjectsEnabled(originalProjectsEnabled); - env.setAllowedSymlinks(new HashSet<>()); - if (repository1 != null) { repository1.destroy(); } @@ -163,8 +163,14 @@ public static void tearDownClass() { } } + @AfterEach + public void tearDown() { + env.setProjectsEnabled(savedProjectsEnabled); + env.setAllowedSymlinks(savedAllowedSymlinks); + } + @Test - public void testSearch() throws IOException, InvalidTokenOffsetsException { + public void testSearch() throws IOException { SearchEngine instance = new SearchEngine(); instance.setFreetext("Hello"); instance.setFile("renamed2.c"); @@ -173,19 +179,18 @@ public void testSearch() throws IOException, InvalidTokenOffsetsException { String[] frags = getFirstFragments(instance); assertNotNull(frags, "getFirstFragments() should return something"); assertEquals(1, frags.length, "frags should have one element"); - assertNotNull("frags[0] should be defined", frags[0]); + assertNotNull(frags[0], "frags[0] should be defined"); final String CTX = - "16
" + - "17" + - " printf ( "Hello, world!\\n" );
" + - "18
"; - assertLinesEqual("ContextFormatter output", CTX, frags[0]); + "16
" + + "17" + + " printf ( "Hello, world!\\n" );
" + + "18
"; + assertLinesEqual(CTX, frags[0], "ContextFormatter output"); instance.destroy(); } - private String[] getFirstFragments(SearchEngine instance) - throws IOException, InvalidTokenOffsetsException { + private String[] getFirstFragments(SearchEngine instance) throws IOException { ContextArgs args = new ContextArgs((short) 1, (short) 10); @@ -197,8 +202,7 @@ private String[] getFirstFragments(SearchEngine instance) AbstractAnalyzer anz = fac.getAnalyzer(); ContextFormatter formatter = new ContextFormatter(args); - OGKUnifiedHighlighter uhi = new OGKUnifiedHighlighter(env, - instance.getSearcher(), anz); + OGKUnifiedHighlighter uhi = new OGKUnifiedHighlighter(instance.getSearcher(), anz); uhi.setBreakIterator(StrictLineBreakIterator::new); uhi.setFormatter(formatter); uhi.setTabSize(TABSIZE); diff --git a/opengrok-indexer/src/test/java/org/opengrok/indexer/util/CustomAssertions.java b/opengrok-indexer/src/test/java/org/opengrok/indexer/util/CustomAssertions.java index f50e320670c..ee24b331a46 100644 --- a/opengrok-indexer/src/test/java/org/opengrok/indexer/util/CustomAssertions.java +++ b/opengrok-indexer/src/test/java/org/opengrok/indexer/util/CustomAssertions.java @@ -51,25 +51,25 @@ protected CustomAssertions() { /** * Asserts the specified strings have equal contents, comparing line-wise * after splitting on LFs. - * @param messagePrefix a message prefixed to line-specific or length- - * specific errors * @param expected the expected content * @param actual the actual content + * @param messagePrefix a message prefixed to line-specific or length- + * specific errors */ - public static void assertLinesEqual(String messagePrefix, String expected, String actual) { + public static void assertLinesEqual(String expected, String actual, String messagePrefix) { String[] expecteds = expected.split("\n"); String[] gotten = actual.split("\n"); - assertLinesEqual(messagePrefix, expecteds, gotten); + assertLinesEqual(expecteds, gotten, messagePrefix); } /** * Asserts the specified lines arrays have equal contents. - * @param messagePrefix a message prefixed to line-specific or length- - * specific errors * @param expecteds the expected content of lines * @param actuals the actual content of lines + * @param messagePrefix a message prefixed to line-specific or length- + * specific errors */ - public static void assertLinesEqual(String messagePrefix, String[] expecteds, String[] actuals) { + public static void assertLinesEqual(String[] expecteds, String[] actuals, String messagePrefix) { List diffLines = new ArrayList<>(); @@ -137,7 +137,7 @@ public static void assertSymbolStream(Class klass, count = 0; for (String token : tokens) { - // 1-based offset to accord with line # + // 1-based index to accord with line # if (count >= expectedTokens.size()) { printTokens(tokens); assertTrue(count < expectedTokens.size(), "too many tokens at term" + (1 + count) + ": " + token); diff --git a/opengrok-indexer/src/test/java/org/opengrok/indexer/web/SearchHelperTest.java b/opengrok-indexer/src/test/java/org/opengrok/indexer/web/SearchHelperTest.java index e9fdc041a88..a0ddad7e3f2 100644 --- a/opengrok-indexer/src/test/java/org/opengrok/indexer/web/SearchHelperTest.java +++ b/opengrok-indexer/src/test/java/org/opengrok/indexer/web/SearchHelperTest.java @@ -82,7 +82,7 @@ private SearchHelper getSearchHelper(String searchTerm) { env.getDataRootFile(), env.getSourceRootFile(), env.getHitsPerPage(), null, new QueryBuilder().setFreetext(searchTerm), false, - env.getUrlPrefix(), false, false); + null, env.getUrlPrefix(), false, false); assertNotSame(0, sh.getBuilder().getSize()); return sh; @@ -93,7 +93,7 @@ private SearchHelper getSearchHelperPath(String searchTerm) { env.getDataRootFile(), env.getSourceRootFile(), env.getHitsPerPage(), null, new QueryBuilder().setPath(searchTerm), false, - env.getUrlPrefix(), false, false); + null, env.getUrlPrefix(), false, false); assertNotSame(0, sh.getBuilder().getSize()); return sh; diff --git a/opengrok-web/src/main/java/org/opengrok/web/PageConfig.java b/opengrok-web/src/main/java/org/opengrok/web/PageConfig.java index b8a211a2ac4..8dd586ce81d 100644 --- a/opengrok-web/src/main/java/org/opengrok/web/PageConfig.java +++ b/opengrok-web/src/main/java/org/opengrok/web/PageConfig.java @@ -83,6 +83,7 @@ import org.opengrok.indexer.index.IndexDatabase; import org.opengrok.indexer.logger.LoggerFactory; import org.opengrok.indexer.search.QueryBuilder; +import org.opengrok.indexer.search.context.ContextArgs; import org.opengrok.indexer.util.IOUtils; import org.opengrok.indexer.util.LineBreaker; import org.opengrok.indexer.util.TandemPath; @@ -1565,6 +1566,7 @@ public SearchHelper prepareInternalSearch(SortOrder sortOrder) { String xrValue = req.getParameter(QueryParameters.NO_REDIRECT_PARAM); return new SearchHelper(getSearchStart(), sortOrder, getDataRoot(), new File(getSourceRootPath()), getSearchMaxItems(), getEftarReader(), getQueryBuilder(), getPrefix() == Prefix.SEARCH_R, + getLimitedContextArgs(), req.getContextPath(), getPrefix() == Prefix.SEARCH_R || getPrefix() == Prefix.SEARCH_P, xrValue != null && !xrValue.isEmpty()); } @@ -1851,7 +1853,7 @@ public boolean evaluateMatchOffset() { breaker.reset(streamSource, in -> ExpandTabsReader.wrap(in, getProject())); int matchLine = breaker.findLineIndex(matchOffset); if (matchLine >= 0) { - // Convert to 1-based offset to accord with OpenGrok line number. + // Convert to 1-based index to accord with OpenGrok line number. fragmentIdentifier = String.valueOf(matchLine + 1); return true; } @@ -1864,4 +1866,59 @@ public boolean evaluateMatchOffset() { } return false; } + + /** + * Gets context settings as specified by the user if available or else using + * the configured defaults -- but also bound by + * {@link ContextArgs#MAX_CONTEXT_SURROUND}. + * @return a defined instance + */ + public ContextArgs getLimitedContextArgs() { + int userContextSurround = getIntParam(QueryParameters.CONTEXT_SURROUND_PARAM, -1); + short contextSurround; + if (userContextSurround < 0) { + contextSurround = env.getContextSurround(); + } else { + contextSurround = userContextSurround <= Short.MAX_VALUE ? + (short) userContextSurround : Short.MAX_VALUE; + } + + if (contextSurround < 0) { + contextSurround = 0; + } else if (contextSurround > ContextArgs.MAX_CONTEXT_SURROUND) { + contextSurround = ContextArgs.MAX_CONTEXT_SURROUND; + } + + short scaledContextLimit = scaleContextLimit(contextSurround, env.getContextLimit()); + return new ContextArgs(contextSurround, scaledContextLimit); + } + + private short scaleContextLimit(short contextSurround, short contextLimit) { + if (contextSurround <= 0) { + return contextLimit; + } + /* + * I experimentally arrived at formula below which scales the + * surrounding context at a slowing rate until then growing linearly + * when sizing for roughly a single match with its large context. I've + * actually decided to cap the maximum surrounding context (elsewhere), + * so the max() is probably not transformative -- but leaving it + * doesn't hurt in case the cap is lifted. + * + * E.g. for contextLimit=10, the scaled limit as contextSurround grows: + * 1 -> 15 2 -> 18 3 -> 20 4 -> 21 + * ... + * 10 -> 25 11 -> 25 12 -> 26 + * 13 -> 27 [max() is transformative.] + * 14 -> 29 [max() is transformative.] + * ... + */ + double scale = 1 + Math.log10(3 * contextSurround); + long limit = Math.round(scale * contextLimit); + limit = Math.max(limit, 2 * contextSurround + 1); + if (limit > Short.MAX_VALUE) { + return Short.MAX_VALUE; + } + return (short) limit; + } } diff --git a/opengrok-web/src/main/webapp/default/print-1.0.2.css b/opengrok-web/src/main/webapp/default/print-1.1.0.css similarity index 91% rename from opengrok-web/src/main/webapp/default/print-1.0.2.css rename to opengrok-web/src/main/webapp/default/print-1.1.0.css index c37db045d72..890fbf7ab08 100644 --- a/opengrok-web/src/main/webapp/default/print-1.0.2.css +++ b/opengrok-web/src/main/webapp/default/print-1.1.0.css @@ -187,14 +187,13 @@ input.submit { /* start search button */ } /* ############### end of header ############## */ - /* ############### start of content ############## */ + #content { position: static; overflow-x: hidden; } - /* *** help page *** */ #help h4 { font-style: italic; @@ -216,13 +215,22 @@ input.submit { /* start search button */ /* *** more page ***/ #more { - line-height: 1.25em; + line-height: 1.5em; } #more b { /* highlight matches */ background-color: #e5e5e5; } +#more .ovl, #results .ovl { + /* + * overline non-consecutive, contextual, "more" line contents or search- + * result line contents + */ + border-style: dotted none none none; + border-width: 1px; + display: block; +} /* *** history page *** */ table#revisions { @@ -259,6 +267,10 @@ table#revisions { text-align: center; } +.rev-message-hidden { + display: none; +} + .rssbadge { /* RSS/XML Feed on history page */ text-align: right; margin: 1ex 0; @@ -320,19 +332,41 @@ table#dirlist { /* the "Name" column */ border-collapse: collapse; } +table#dirlist thead tr:nth-child(1) { + height: 22px; +} + #dirlist .r, #dirlist .p { padding: 0; margin: 0 0 0 1em; } -#dirlist td:nth-child(n+2) { /* all but the first column */ +#dirlist td:nth-child(n+3) { /* all but the first 2 columns */ padding-right: 1em; } -#dirlist tbody td:nth-child(4) { +#dirlist tbody td:nth-child(5) { text-align: right; /* CSS3 may allow " " (single space char) */ } +#dirlist td.q { /* 2nd column: H A D */ + white-space: nowrap; + font-size: small; + padding-left: 16px; + width: 3em; +} + +#dirlist td.numlines { /* 5th column: #Lines */ + text-align: right; /* CSS3 may allow " " (single space char) */ +} + +#dirlist td.loc { /* 6th column: LOC */ + text-align: right; /* CSS3 may allow " " (single space char) */ +} + +span.simplified-path { + color: #888; +} /* file display */ #src { @@ -365,6 +399,12 @@ table#dirlist { /* the "Name" column */ color: #000; } +/* highlight line number with anchor */ +#src a.l:target, #src a.hl:target { + background: orange; + color:yellow; +} + .blame .r { /* revision number "column" (annotation) */ text-align: right; } @@ -373,6 +413,10 @@ table#dirlist { /* the "Name" column */ text-align: center; } +.most_recent_revision { + font-weight: bold; +} + /* source code highlighting - see org/opengrok/analysis/$lang/*Xref.lex */ #src .n { /* numbers/label */ color: #a52a2a; } #src .s { /* strings */ color: green; } @@ -504,10 +548,6 @@ a.xsr { /* subroutine */ color: #00f; font-weight: bold; } /* ############### end of footer ############## */ -.important-note { - display: none; -} - /* *** scopes *** */ span.scope-head { @@ -537,3 +577,7 @@ span.scope-signature { div#scope { display: none; } + +.important-note { + display: none; +} diff --git a/opengrok-web/src/main/webapp/default/style-1.0.4.css b/opengrok-web/src/main/webapp/default/style-1.1.0.css similarity index 96% rename from opengrok-web/src/main/webapp/default/style-1.0.4.css rename to opengrok-web/src/main/webapp/default/style-1.1.0.css index 81ee3061054..93303a7fa9c 100644 --- a/opengrok-web/src/main/webapp/default/style-1.0.4.css +++ b/opengrok-web/src/main/webapp/default/style-1.1.0.css @@ -104,6 +104,11 @@ button:hover, button:active, .btn:hover, .btn:active { label { } +label.sch { + /* Surrounding context chooser */ + font-size: small; +} + .pre { /* the diff content */ white-space: pre-wrap; font-family: monospace; @@ -500,13 +505,40 @@ html.history #content{ /* *** more page ***/ #more { - line-height: 1.25em; + line-height: 1.5em; } #more b { /* highlight matches */ background-color: #ffff99; } +#more .ovl, #results .ovl { + /* + * overline non-consecutive, contextual, "more" line contents or search- + * result line contents. + */ + border-style: dotted none none none; + border-width: 1px; + display: block; + white-space: pre; + /* Safari work-around re word-wrap when white-space: pre is used. */ + word-wrap: normal; +} + +#more .xovl, #results .xovl { + /* + * Applies for contextual search result line contents that are not #ovl. + * Technically #more is already in a 
 but avoiding a missing style.
+     */
+    white-space: pre;
+    /* Safari work-around re word-wrap when white-space: pre is used. */
+    word-wrap: normal;
+}
+
+#more .ovl b, #results .ovl b, #more .xovl b, #results .xovl b {
+    /* highlight matches when surrounding context is shown. */
+    background-color: #ffff99;
+}
 
 /* *** history page *** */
 table#revisions {
diff --git a/opengrok-web/src/main/webapp/httpheader.jspf b/opengrok-web/src/main/webapp/httpheader.jspf
index 4e96c7f95a3..708890ab902 100644
--- a/opengrok-web/src/main/webapp/httpheader.jspf
+++ b/opengrok-web/src/main/webapp/httpheader.jspf
@@ -45,8 +45,8 @@ org.opengrok.web.Scripts"
     PageConfig cfg = PageConfig.get(request);
     String styleDir = cfg.getCssDir();
     String ctxPath = request.getContextPath();
-    String dstyle = styleDir + '/' + "style-1.0.4.min.css";
-    String pstyle = styleDir + '/' + "print-1.0.2.min.css";
+    String dstyle = styleDir + '/' + "style-1.1.0.min.css";
+    String pstyle = styleDir + '/' + "print-1.1.0.min.css";
     String mstyle = styleDir + '/' + "mandoc-1.0.0.min.css";
 %>
 Project(s)
     
     
-    
@@ -156,7 +157,7 @@ document.domReady.push(function() { domReadyMenu(); });
     
         
-        
@@ -164,7 +165,7 @@ document.domReady.push(function() { domReadyMenu(); });
     
         
-        
@@ -172,7 +173,7 @@ document.domReady.push(function() { domReadyMenu(); });
     
         
-        
@@ -183,7 +184,7 @@ document.domReady.push(function() { domReadyMenu(); });
     
         
-        
@@ -193,7 +194,7 @@ document.domReady.push(function() { domReadyMenu(); });
     %>
     
         
-        <%
                 String selection = queryParams.getType();
                 %>
@@ -216,18 +217,25 @@ document.domReady.push(function() { domReadyMenu(); });
     
 
 

-    
-    
-    
-    
+    
 

 
 

-    
+    <%-- filled with javascript --%>
 

 
diff --git a/opengrok-web/src/main/webapp/minisearch.jspf b/opengrok-web/src/main/webapp/minisearch.jspf
index daf1a0e6eb7..d8196fb9243 100644
--- a/opengrok-web/src/main/webapp/minisearch.jspf
+++ b/opengrok-web/src/main/webapp/minisearch.jspf
@@ -21,10 +21,12 @@ Portions Copyright (c) 2020, Chris Fraire .
 --%>
 <%@ page session="false" errorPage="error.jsp" import="
 org.opengrok.indexer.configuration.Project,
+org.opengrok.indexer.search.context.ContextArgs,
 org.opengrok.web.PageConfig,
 org.opengrok.indexer.web.Prefix,
 org.opengrok.indexer.web.QueryParameters,
-org.opengrok.indexer.web.Util"%><%
+org.opengrok.indexer.web.Util"%>
+<%
     /* ---------------------- minisearch.jspf start --------------------- */
 {
         PageConfig cfg = PageConfig.get(request);
@@ -98,10 +100,20 @@ org.opengrok.indexer.web.Util"%><%
     }
         %>
    • 
-            
    • <%
+        
    • 
+        
  • 
+            
    • <%
     Project proj = cfg.getProject();
     String[] vals = cfg.getSearchOnlyIn();
-        %>
  • 
+            
    • 
     <%
diff --git a/opengrok-web/src/main/webapp/more.jsp b/opengrok-web/src/main/webapp/more.jsp
index 14f8cf33020..d22a047617a 100644
--- a/opengrok-web/src/main/webapp/more.jsp
+++ b/opengrok-web/src/main/webapp/more.jsp
@@ -1,6 +1,4 @@
 <%--
-$Id$
-
 CDDL HEADER START
 
 The contents of this file are subject to the terms of the
@@ -84,14 +82,14 @@ file="mast.jsp"
         Query tquery = qbuilder.build();
         if (tquery != null) {
 %>
    Lines Matching <%= tquery %>
    
-
    
+
    
     
    <%
             String xrefPrefix = request.getContextPath() + Prefix.XREF_P;
             boolean didPresentNew = false;
             if (docId >= 0) {
-                didPresentNew = searchHelper.getSourceContext().getContext2(env,
-                    searchHelper.getSearcher(), docId, out, xrefPrefix, null, false,
-                    tabSize);
+                didPresentNew = searchHelper.getSourceContext().getContext2(
+                        searchHelper.getSearcher(), docId, out, xrefPrefix, null,
+                        searchHelper.getUnlimitedContextArgs(), tabSize);
             }
             if (!didPresentNew) {
                 /**
diff --git a/opengrok-web/src/main/webapp/search.jsp b/opengrok-web/src/main/webapp/search.jsp
index 6d8e60286a7..06d80b5f6b3 100644
--- a/opengrok-web/src/main/webapp/search.jsp
+++ b/opengrok-web/src/main/webapp/search.jsp
@@ -19,10 +19,8 @@ CDDL HEADER END
 Copyright (c) 2005, 2021, Oracle and/or its affiliates. All rights reserved.
 Portions Copyright 2011 Jens Elkner.
 Portions Copyright (c) 2017-2018, 2020, Chris Fraire .
-
 --%>
 <%@page contentType="text/html; charset=UTF-8" pageEncoding="UTF-8"%>
-<%@page import="jakarta.servlet.http.HttpServletResponse"%>
 <%@page session="false" errorPage="error.jsp" import="
 org.apache.lucene.queryparser.classic.QueryParser,
 org.opengrok.indexer.search.Results,
@@ -32,11 +30,12 @@ org.opengrok.indexer.web.SearchHelper,
 org.opengrok.indexer.web.SortOrder,
 org.opengrok.indexer.web.Suggestion,
 
-java.util.List"
+java.nio.charset.StandardCharsets,
+java.util.List,
+jakarta.servlet.http.Cookie,
+jakarta.servlet.http.HttpServletRequest,
+jakarta.servlet.http.HttpServletResponse"
 %>
-<%@ page import="jakarta.servlet.http.HttpServletRequest" %>
-<%@ page import="jakarta.servlet.http.Cookie" %>
-<%@ page import="java.nio.charset.StandardCharsets" %>
 <%
 {
     PageConfig cfg = PageConfig.get(request);