Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Show search notebook link in handbook and blog #4899

Closed
wants to merge 8 commits into from
Closed

Conversation

jyavorska
Copy link
Contributor

@jyavorska jyavorska commented Sep 30, 2021

This makes it so if you include a Sourcegraph notebook code fence in your markdown, it will add a link to the search page. For this iteration, it does not try to render a control to embed a working search on the page, just makes it so you can get to the search easily. Styling is absolutely minimal just to prove it works.

For example, using the following in markdown:

```sourcegraph
patterntype:structural repo:^github\.com/gitpod-io/openvscode-server$@5c8a1f file:^src/vs/code/browser/workbench/workbench\.ts create(document.body, {:[1]})

Will render as the following in the handbook:

image

Or as follows on the site:

image

Notes

  • There are sample pages at http://localhost:9000/blog/test/ and http://localhost:5082/handbook/product/test included in this PR
  • There is various test content that needs to be removed.
  • It required a change to github.com/sourcegraph/docsite to re-enable the wrapping pre, since otherwise no CSS classes indicating the language that was being rendered was available, and this required a minor CSS addition to ensure it rendered similarly to how it looks today. See https://github.com/jyavorska/sourcegraph for the fork, and layout.css for the handbook CSS adjustment. There's also an adjustment in the go.mod in this repo to replace the official docsite repo with my fork that needs to be removed if/when the main sourcegraph docsite repo is updated.
  • I don't know react and couldn't figure out the right place to put the Javascript for the main website so it would run. It's on the page for now, but that should be easily fixable. The handbook has it in the correct central location.
  • The script and CSS are copied in both the handbook and main website areas, even though they are exactly the same in the case of the Javascript, and could be made to be the same for the CSS. Not sure if there's an easy way to share it between them but if so that would be good.
  • No tests :)

@sourcegraph-bot
Copy link
Contributor

sourcegraph-bot commented Sep 30, 2021

Notifying subscribers in CODENOTIFY files for diff 9314328...74a62a7.

Notify File(s)
@christinaforney handbook/product/index.md
handbook/product/test.md
@sourcegraph/marketing blogposts/2021/test.md
website/src/css/components/_SearchNotebook.scss
website/src/css/styles.scss

@jyavorska
Copy link
Contributor Author

Cc @benvenker @lguychard not sure what you all think of this. It's very hacky. :)

@@ -276,3 +276,8 @@ button#CybotCookiebotDialogBodyButtonDetails {
.container-md {
max-width: 1140px;
}

/* Fix so that turning off outer pre does not impact styling */
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps not really necessary? There's just a bit of extra padding around syntax highlighting without it, that isn't there on about.sourcegraph.com today because of re-adding the wrapping pre.

@@ -0,0 +1,31 @@
---
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file should be deleted before merging.

@@ -8,3 +8,5 @@ require (
github.com/sourcegraph/docsite v1.6.0 // indirect
golang.org/x/oauth2 v0.0.0-20200107190931-bf48bf16ab8d
)

replace github.com/sourcegraph/docsite => github.com/jyavorska/docsite v1.8.3-0.20210930071952-c6a6c94c17fe
Copy link
Contributor Author

@jyavorska jyavorska Sep 30, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line should be deleted (and the docsite version incremented) if github.com/sourcegraph/docsite is updated to stop supressing the wrapping pre.

@@ -8,6 +8,10 @@ The Product team strives to make the following true:
- Each teammate has the customer and product context needed (about customer problems, likely future priorities, possible solutions, etc.) to perform their work effectively.
- The product vision and roadmap are communicated well to teammates and everyone outside Sourcegraph.

## Test page, please remove
Copy link
Contributor Author

@jyavorska jyavorska Sep 30, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Delete this section before merging.

@@ -0,0 +1,9 @@
# Test page
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Delete this file before merging.

@jyavorska jyavorska changed the title Add link to search notebook for handbook and blog Show search notebook link in handbook and blog Sep 30, 2021
def hello
```

<script>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be moved to some reused place.

@lguychard
Copy link
Contributor

@jyavorska great idea! I think that, unfortunately, the current implementation is likely to be broken by #hacking-handbook-conversion-nextjs (working doc), which is currently in progress.

On the flipside, when we get a NextJS-backed handbook, we also get mdx support, which means we can embed React components within Markdown files. Thus, we can more easily render notebooks component directly in the handbook instead of just linking to Sourcegraph!

@Joelkw
Copy link
Contributor

Joelkw commented Sep 30, 2021

This is awesome! (Feels like one of those obvious "of course we have this" things that we don't yet have!)

I'll defer to everyone else on the outcome re: the handbook migration, but @jyavorska just wanted to note that the Sourcegraph docs use ```sgquery ... ``` instead of the ```sourcegraph ... ``` you set up here. I don't have a strong preference but was wondering if we should consider unifying that? Examples can be found here: https://sourcegraph.com/search?q=context:%40sourcegraph/all+repo:%5Egithub%5C.com/sourcegraph/sourcegraph%24+file:%5Edoc+%60%60%60sgquery&patternType=literal

I only just learned about ```sgquery two weeks ago, and I don't think even people who've been here many years longer than me are aware of it since they also learned about it then, so unifying them might help build awareness.

@jyavorska
Copy link
Contributor Author

@

This is awesome! (Feels like one of those obvious "of course we have this" things that we don't yet have!)

I'll defer to everyone else on the outcome re: the handbook migration, but @jyavorska just wanted to note that the Sourcegraph docs use ```sgquery ... ``` instead of the ```sourcegraph ... ``` you set up here. I don't have a strong preference but was wondering if we should consider unifying that? Examples can be found here: https://sourcegraph.com/search?q=context:%40sourcegraph/all+repo:%5Egithub%5C.com/sourcegraph/sourcegraph%24+file:%5Edoc+%60%60%60sgquery&patternType=literal

I only just learned about ```sgquery two weeks ago, and I don't think even people who've been here many years longer than me are aware of it since they also learned about it then, so unifying them might help build awareness.

Thanks @Joelkw! I based off that recent VS Code article, and they used sourcegraph and it works (https://raw.githubusercontent.com/gitpod-io/openvscode-server/main/docs/sourcedive.snb.md) but it should be whatever is actually correct for sure. Presumably the Search team can let us know.

@jyavorska jyavorska closed this Oct 4, 2021
@jyavorska
Copy link
Contributor Author

This PR is not mergeable due to the handbook refactor, closing.

@rebeccadee
Copy link
Contributor

@csbailey5t Is this what you were hoping we would be able to do (with the Paul Jolly/CUE post)?

@csbailey5t
Copy link
Contributor

@csbailey5t Is this what you were hoping we would be able to do (with the Paul Jolly/CUE post)?

This is the first step of that. The goal would be being able to run the search notebook code blogs on the blog page, rather than linking out to the notebook on Sourcegraph.

@bretthayes bretthayes deleted the jason-hackathon branch May 13, 2022 18:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants