rustdoc search: add new crate: syntax to search a single crate

[lolbinarycat]

alternative to #129769

fixes #129537

limitations: currently, the crate filter has to be followed by a comma, so it's crate:std, vec.

r? @notriddle

[rustbot]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @notriddle (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

• @rustbot author: the review is finished, PR author should check the comments and take action accordingly
• @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

[rustbot]

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez, @jsha

[notriddle]

This seems great! 👍 You can work around the comma thing by using an inelegant special case in the parser.

diff --git a/src/librustdoc/html/static/js/search.js b/src/librustdoc/html/static/js/search.js
index 35ba2f50055..f02c488b530 100644
--- a/src/librustdoc/html/static/js/search.js
+++ b/src/librustdoc/html/static/js/search.js
@@ -283,6 +283,20 @@ function getFilteredNextElem(query, parserState, elems, isInGenerics) {
         parserState.pos += 1;
         parserState.totalElems -= 1;
         query.literalSearch = false;
+        if (parserState.typeFilter === "crate") {
+            while (parserState.userQuery[parserState.pos] === " ") {
+                parserState.pos += 1;
+            }
+            const start = parserState.pos;
+            const foundCrate = consumeIdent(parserState);
+            if (!foundCrate) {
+                throw ["Expected ident after ", "crate:", ", found ", parserState.userQuery[start]];
+            }
+            const name = parserState.userQuery.substring(start, parserState.pos);
+            elems.push(makePrimitiveElement(name, { typeFilter: "crate" }));
+            parserState.typeFilter = null;
+            return getFilteredNextElem(query, parserState, elems, isInGenerics);
+        }
         getNextElem(query, parserState, elems, isInGenerics);
     }
 }

[notriddle]

@rfcbot poll

[rfcbot]

Team member @notriddle has asked teams: T-rustdoc-frontend, for consensus on:

• @GuillaumeGomez
• @camelid
• @jsha
• @notriddle

[GuillaumeGomez]

I personally prefer the approach in #129769.

[lolbinarycat]

@GuillaumeGomez i prefer this approach due to it not breaking existing workflows.

perhaps in the future i'll add a completion menu when typing crate:, sort of a hybrid approach.

[GuillaumeGomez]

I think adding the possibility to filter on multiple crates and therefore allowing to repeat crate: (or a different syntax, like crate names separated with commas?) would be a good thing. Better for ys to decide exactly on what we want and how the syntax should look like too before approving this syntax extension.

[lolbinarycat]

@GuillaumeGomez searching multiple crates would be nice (i think it should accept either specifying it multiple times or separated by commas, should be pretty easy to implement into the parser), however it would require more invasive changes, since the code assumes filterCrates is either null or a string, not an array. i can do this, but i would prefer to do it in a followup PR, to prevent having to deal with too many merge conflicts.

[GuillaumeGomez]

Considering it will impact the search syntax, better first to all agree to what we want before making any changes. You can make changes (once it's agreed upon, otherwise it could be work for nothing, which isn't great) in new commits, it's fine.

[notriddle]

Considering it will impact the search syntax, better first to all agree to what we want before making any changes.

I disagree. Shipping an MVP that only works with one crate sounds fine, as long as things that work now continue to work in the future (gratuitous breakage is bad, but rustdoc search isn't bound by the extremely strict guarantees of the compiler).

[lolbinarycat]

self-review: the type declarations in externs.js need to be updated before this gets merged.

[bors]

☔ The latest upstream changes (presumably #127589) made this pull request unmergeable. Please resolve the merge conflicts.

[jsha]

Two UI bugs from trying a demo (testing on Chrome / Linux):

• After clicking a specific crate from the drop-down, clicking "all crates" does nothing.
• If the "Directly go to item in search if there is only one result" checkbox is set, selecting a crate in the drop-down can immediately navigate to a different page if there's only one result. While I could see an argument that this behavior "does what it says in the setting," I think it's quite surprising and isn't inline with the intended use of that checkbox, which is to quickly navigate to a specific known thing. By the time someone is looking at a page of search results, changing the crate dropdown shouldn't activate the "immediately navigate" code.

Overall the concept is good so I'm replying affirmatively to the poll.

[jsha]

A syntax problem:

crate:proc_macro fn:fmt works, but fn:fmt crate:proc_macro produces Query parser error: "Expected ,, : or ->, found :".

[lolbinarycat]

If the "Directly go to item in search if there is only one result" checkbox is set, selecting a crate in the drop-down can immediately navigate to a different page if there's only one result. While I could see an argument that this behavior "does what it says in the setting," I think it's quite surprising and isn't inline with the intended use of that checkbox, which is to quickly navigate to a specific known thing. By the time someone is looking at a page of search results, changing the crate dropdown shouldn't activate the "immediately navigate" code.

are you sure that's not existing behavior? I can't come up with a query to reproduce this.

[lolbinarycat]

crate:proc_macro fn:fmt works, but fn:fmt crate:proc_macro produces Query parser error: "Expected ,, : or ->, found :".

Honestly I don't know if this is easily fixable, even the ability to not have a comma in the first case is a bit of a hack.

[notriddle]

I think that could be fixed by tweaking isPathSeparator. Right now, it operates on a single character:

rust/src/librustdoc/html/static/js/search.js

Lines 197 to 217 in 2f3708a

	/**
	* Returns `true` if the given `c` character is a separator.
	*
	* @param {string} c
	*
	* @return {boolean}
	*/
	function isSeparatorCharacter(c) {
	return c === "," || c === "=";
	}
	/**
	* Returns `true` if the current parser position is starting with "->".
	*
	* @param {ParserState} parserState
	*
	* @return {boolean}
	*/
	function isReturnArrow(parserState) {
	return parserState.userQuery.slice(parserState.pos, parserState.pos + 2) === "->";
	}

If it were tweaked to work more like isReturnArrow, it could treat the phrase "[space] crate:" as a separator.

[lolbinarycat]

@notriddle I tried adjusting isPathSeperator, isSeparatorCharacter, and getIdentEndPosition, none of them worked.

personally, adding a bunch of fragile special cases just to syntactically overload doesn't feel like the best idea. If we want to change the semantic meaning of spaces (as suggested by jsha), we should do that in a principled, holistic, well planned action, instead of just adding ad-hoc exceptions.

I think we should just document that not requiring a comma when crate: appears at the start of a search is a special case, and using it in other contexts requires adding a comma. Indeed, I'm pretty sure it isn't supported if it shows up in the return type list.