Skip to content

Conversation

nickvigilante
Copy link

@nickvigilante nickvigilante commented Oct 24, 2024

Welcome to the quarto GitHub repo!

We are always happy to hear feedback from our users.

To file a pull request, please follow these instructions carefully: https://yihui.org/issue/#bug-reports

If you're a collaborator from outside quarto-dev making changes larger than a typo, please make sure you have filed an individual or corporate contributor agreement. You can send the signed copy to [email protected].

Also, please complete and keep the checklist below.

Description

This PR adds a class, quarto-exclude-from-search-index, that you can add to any div or span to exclude the content from that section from Quarto's search index (search.json). This PR fixes #11189.

Checklist

I have (if applicable):

  • filed a contributor agreement.
  • referenced the GitHub issue this PR closes
  • updated the appropriate changelog

@cscheid
Copy link
Collaborator

cscheid commented Oct 24, 2024

Thanks!

Let's talk about the class you're using. I would prefer that we used a class starting with quarto-, something likequarto-exclude-from-search-index. It is admittedly verbose, but this is a feature that has the potential to be quite confusing for users if they're unaware, and a quarto- prefix minimizes the change that some existing document already has a class like that. (You can always retarget your preferred classes to quarto-exclude-from-search-index in a Lua filter)

In addition to the CLA signature and the changelog entry, we will need a few more things:

  1. Tests of course need to all pass.
  2. This is a new feature, so we need an accompanying documentation PR against the prerelease branch of https://github.com/quarto-dev/quarto-web/
  3. Tests. Probably the easiest way to do it is to add a test that
  • renders a full project with the a div that excludes a unique word, and then check that the word is missing from the search.json file.
  • renders the same project with a project profile that adds a filter to remove the class, and then check that word is present in the corresponding search.json file.

@nickvigilante
Copy link
Author

The class is now renamed to quarto-exclude-from-search-index. CLA is underway. I contacted the head of ops at TileDB for a review, and I'm also trying to identify other members of my company who may want to contribute to this repo in the future. In response to your other feedback:

  1. Understood, and I'm now testing different variations locally. My TypeScript is rusty, but it'll get there 😄
  2. Once I get this working, I'll start work on a docs update.
  3. I'll work on tests as well.

@cscheid
Copy link
Collaborator

cscheid commented Oct 24, 2024

The class is now renamed to quarto-exclude-from-search-index.

Just FYI, I don't think you've pushed this yet.

@nickvigilante
Copy link
Author

Correct, I haven't. I'm still testing locally.

@cscheid cscheid changed the title Fixes #11189 Allow portions of documents to be excluded from search index Oct 24, 2024
@cscheid
Copy link
Collaborator

cscheid commented Oct 24, 2024

@nickvigilante I was in the process of fixing a separate search index bug, and so I made a PR that includes a test that you should be able to adapt for this one. Take a look at #11191

@nickvigilante nickvigilante force-pushed the fix-11189 branch 6 times, most recently from c0d3ff3 to 20337f9 Compare October 28, 2024 13:52
@cscheid cscheid added this to the v1.7 milestone Nov 18, 2024
- Users will now be able to create `div`s and `span`s that exclude
certain page elements from the search index.
@nickvigilante
Copy link
Author

@cscheid Hey Carlos, sorry I dropped the ball on this. I was scrambling to get stuff done before year end.

I'm trying to get this final smoke test to pass, and I keep hitting an error. Do you have any pointers on how to overcome this?

@cderv
Copy link
Collaborator

cderv commented Jan 14, 2025

@nickvigilante did you try to run your test locally ? This is easier to debug and check that eveyrhing works as you expect.
Do they passe locally for you ?

You could try

quarto render --profile with-filter docs/search/issue-11189

Also try in your test

const withFilterProfile = ["--profile", "with-filter"]

and

testQuartoCmd(
  "render",
  [input, ...withFilterProfile]

so that each args is passed separately and --profile flag correctly identified (and not seen as '--profile with-filter')

You can also use run-test.sh locally if you have test environment setup in dev version.

Hope it helps

@cscheid cscheid added the early-in-release An issue that should be worked on early in the release (likely due to risk) label Apr 4, 2025
@cscheid cscheid modified the milestones: v1.7, v1.8 Apr 4, 2025
@cscheid cscheid self-assigned this Apr 4, 2025
@cscheid cscheid modified the milestones: v1.8, v1.9 Aug 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

early-in-release An issue that should be worked on early in the release (likely due to risk)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add a class that excludes blocks of text from being added to the OOTB Quarto search index (search.json)

3 participants