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

fix: Star-search md rendering renders spacing from LLM #3326

Merged
merged 2 commits into from May 7, 2024

Conversation

jpmcb
Copy link
Member

@jpmcb jpmcb commented May 7, 2024

Description

This adds 2 things primarily to the star-search markdown rendering:

  1. Empty data: frames are counted as newlines from the model. This is done by making the \s spacing capture in the regex optional and not forcing a capture in the results capture-group via .+: instead, we allow for any captures (including zero) through .*.
  2. When we get an empty string in the capture group, adds a "  \n" which seems to be the way to get react-markdown to render some newlines/spacing correctly. I may be wrong on this, but it seems to work okay.

This also changes Markdown --> ReactMarkdown. This seems to be the way the library recommends rendering "safe" markdown that can't have code injected into it: https://www.npmjs.com/package/react-markdown/v/8.0.6

I also noticed what seems to be some improvements to the rendering of the markdown switching to ReactMarkdown.

Related Tickets & Documents

Mobile & Desktop Screenshots/Recordings

Before:

Screenshot 2024-05-06 at 10 44 16 PM

After:

Screenshot 2024-05-06 at 10 47 35 PM

Steps to QA

Tier (staff will fill in)

  • Tier 1
  • Tier 2
  • Tier 3
  • Tier 4

[optional] What gif best describes this PR or how it makes you feel?

Signed-off-by: John McBride <john@opensauced.pizza>
@jpmcb jpmcb requested review from nickytonline, zeucapua and a team May 7, 2024 05:43
Copy link

netlify bot commented May 7, 2024

Deploy Preview for oss-insights ready!

Name Link
🔨 Latest commit 1a2f4d6
🔍 Latest deploy log https://app.netlify.com/sites/oss-insights/deploys/663a123bea7fb900084a57f5
😎 Deploy Preview https://deploy-preview-3326--oss-insights.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented May 7, 2024

Deploy Preview for design-insights ready!

Name Link
🔨 Latest commit 1a2f4d6
🔍 Latest deploy log https://app.netlify.com/sites/design-insights/deploys/663a123bcf8dd70008f1a468
😎 Deploy Preview https://deploy-preview-3326--design-insights.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@jpmcb
Copy link
Member Author

jpmcb commented May 7, 2024

I also came across this plugin: https://github.com/remarkjs/remark-breaks which would add <br> to the content and maybe make it make more sense: it seems this is maybe what GitHub does in its markdown rendering for comments / issues, etc.

Copy link
Member

@nickytonline nickytonline left a comment

Choose a reason for hiding this comment

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

I pushed one change, but this is good to go.

:shipit:

pages/star-search/index.tsx Outdated Show resolved Hide resolved
Copy link
Member

@bdougie bdougie left a comment

Choose a reason for hiding this comment

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

This is much better.

@nickytonline nickytonline merged commit 7bf27f1 into beta May 7, 2024
13 checks passed
@nickytonline nickytonline deleted the star-search-md-formating branch May 7, 2024 15:25
open-sauced bot pushed a commit that referenced this pull request May 7, 2024
## [2.25.0-beta.3](v2.25.0-beta.2...v2.25.0-beta.3) (2024-05-07)

### 🐛 Bug Fixes

* Star-search md rendering renders spacing from LLM ([#3326](#3326)) ([7bf27f1](7bf27f1))
nickytonline added a commit that referenced this pull request May 7, 2024
Signed-off-by: John McBride <john@opensauced.pizza>
Co-authored-by: Nick Taylor <nick@nickyt.co>
open-sauced bot pushed a commit that referenced this pull request May 7, 2024
## [2.25.0](v2.24.2...v2.25.0) (2024-05-07)

### 🐛 Bug Fixes

* Change announcement card on highlight feed to starsearch ([#3313](#3313)) ([a38af76](a38af76))
* Star-search md rendering renders spacing from LLM ([#3326](#3326)) ([7bf27f1](7bf27f1))
* style and layout fixes for StarSearch ([#3304](#3304)) ([599d8f6](599d8f6))

### 🍕 Features

* Issues and PR charts for Repo pages ([#3323](#3323)) ([97a6239](97a6239))
* search and select for workspaces in sidebar ([#3307](#3307)) ([5d91aaf](5d91aaf))
* uniform styling for Repo page charts ([#3306](#3306)) ([9cd008e](9cd008e))
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

3 participants