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

cmd/bsky-webhook: add support for facets #11

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 90 additions & 0 deletions cmd/bsky-webhook/facets.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
package main

import (
"fmt"
"slices"
"strings"
)

func bskyMessageToSlackMarkup(bskyMessage BskyMessage) (string, error) {
Copy link
Member

Choose a reason for hiding this comment

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

An optional general suggestion:

For parameters in small scopes where their meaning is close to its use, it's fine (and indeed conventionally preferable) to choose less verbose names. Since the type is right here, this could be message or even simply msg, there is only one message involved here.

In particular, I recommend against re-stating the type of a declared variable in the name, unless for some reason it makes a distinction (e.g., "fooString" vs. "fooBytes" if we had both formats to deal with)

var slackStringBuilder strings.Builder
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this needs to say so much, there's only one string builder here:

var sb strings.Builder


fragments, err := facetsToFragments(bskyMessage)
if err != nil {
return "", err
}

for _, fragment := range fragments {
if fragment.Features == nil {
slackStringBuilder.WriteString(fragment.Text)
} else {
uri := ""
for _, feature := range fragment.Features {
if feature.Type == "app.bsky.richtext.facet#link" {
uri = feature.Uri
break
} else if feature.Type == "app.bsky.richtext.facet#mention" {
uri = fmt.Sprintf("https://bsky.app/profile/%s", feature.Did)
break
} else if feature.Type == "app.bsky.richtext.facet#tag" {
uri = fmt.Sprintf("https://bsky.app/hashtag/%s", feature.Tag)
}
}
if uri != "" {
slackStringBuilder.WriteString(fmt.Sprintf("<%s|%s>", uri, fragment.Text))
} else {
slackStringBuilder.WriteString(fragment.Text)
}
}
Comment on lines +18 to +38
Copy link
Member

Choose a reason for hiding this comment

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

I think this loop could be simplified quite a bit with a helper method:

func (b BskyFacet) featureURI() string {
   for _, feat := range b.Features {
      switch feat.URI {
      case "app.bsky.richtext.facet#link":
         return feat.URI
      case "app.bsky.richtext.facet#mention":
         return fmt.Sprintf("https://bsky.app/profile/%s", feat.DID)
      case "app.bsky.richtext.facet#tag":
         return fmt.Sprintf("https://bsky.app/hashtag/%s", feat.Tag)
      }
   }
   return ""
}

Then the whole body of the (outer) loop can be something like:

for _, frag := range fragments {
   if uri := frag.featureURI(); uri != "" {
      fmt.Fprintf(&sb, "<%s|%s>", uri, frag.Text)
   } else {
      sb.WriteString(frag.Text)
   }
}

(Note as illustrated here, you can use the builder as an io.Writer and format directly into it)

}

return slackStringBuilder.String(), nil
}

func facetsToFragments(bskyMessage BskyMessage) ([]BskyTextFragment, error) {
facets := bskyMessage.Commit.Record.Facets
textString := bskyMessage.Commit.Record.Text

fragments := []BskyTextFragment{}

slices.SortStableFunc(facets, func(a, b BskyFacet) int {
return a.Index.ByteStart - b.Index.ByteStart
Copy link
Member

Choose a reason for hiding this comment

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

return cmp.Compare(a.Index.ByteStart, b.Index.ByteStart)

Copy link
Member

Choose a reason for hiding this comment

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

Though if you really care about stability, maybe what we want here is a tie breaker, e.g.,

if c := cmp.Compare(a.Index.ByteStart, b.Index.ByteStart); c != 0 {
   return c
}
return cmp.Compare(b.Index.ByteEnd, a.Index.ByteEnd)

or something like that (so they have proper nesting order within the ties)

YMMV.

})

textCursor := 0
Copy link
Member

@creachadair creachadair Jan 3, 2025

Choose a reason for hiding this comment

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

If I understand correctly, the delicate dance with indices here is something to do with the fact that facets are specified by offsets into the base text?

Given that you went to the trouble of ordering them above, maybe this could be turned into a helper that uses slices.BinarySearch (or even just scans them linearly—there aren't going to be that many, we don't need to worry about asymptotics I think) to find the relevant fragment for each facet? (Even if we do care, we already spent O(n lg n) to sort them, so spending another n lg n to probe them won't affect the underlying shape)

I am sort of convinced this does what I described above, but index dancing is hard to follow, so I would suggest we try to be more obvious here.

(Then too, I think maybe we don't need to do any additional appending, since the facet decorations could be glued directly into place on the facet fragment they belong to I think?)

facetCursor := 0

for facetCursor < len(facets) {
currentFacet := facets[facetCursor]

if textCursor < currentFacet.Index.ByteStart {
fragments = append(fragments, BskyTextFragment{Text: textString[textCursor:currentFacet.Index.ByteStart]})
} else if textCursor > currentFacet.Index.ByteStart {
facetCursor++
continue
}

if currentFacet.Index.ByteStart < currentFacet.Index.ByteEnd {
fragmentText := textString[currentFacet.Index.ByteStart:currentFacet.Index.ByteEnd]

// dont add the features if the text is blank
if strings.TrimSpace(fragmentText) == "" {
fragments = append(fragments, BskyTextFragment{
Text: textString[currentFacet.Index.ByteStart:currentFacet.Index.ByteEnd],
})
} else {
fragments = append(fragments, BskyTextFragment{
Text: textString[currentFacet.Index.ByteStart:currentFacet.Index.ByteEnd],
Features: currentFacet.Features,
})
}
}
textCursor = currentFacet.Index.ByteEnd
facetCursor++
}
if textCursor < len(textString) {
fragments = append(fragments, BskyTextFragment{Text: textString[textCursor:]})
}

return fragments, nil
}
14 changes: 13 additions & 1 deletion cmd/bsky-webhook/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -288,12 +288,24 @@ func getBskyProfile(ctx context.Context, bskyMessage BskyMessage, bsky *bluesky.
}

func sendToSlack(ctx context.Context, jetstreamMessageStr string, bskyMessage BskyMessage, imageURL string, profile bluesky.Profile, postTime time.Time) error {
var messageText string
var err error

if len(bskyMessage.Commit.Record.Facets) != 0 {
messageText, err = bskyMessageToSlackMarkup(bskyMessage)
if err != nil {
return err
}
} else {
messageText = bskyMessage.Commit.Record.Text
Copy link
Member

Choose a reason for hiding this comment

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

Could maybe have bskyMessageToSlackMarkup handle this case so you don't have to check for it here? (Actually I think it might already)

}

attachments := []SlackAttachment{
{
AuthorName: fmt.Sprintf("%s (@%s)", profile.Name, profile.Handle),
AuthorIcon: profile.AvatarURL,
AuthorLink: fmt.Sprintf("https://bsky.app/profile/%s", profile.Handle),
Text: fmt.Sprintf("%s\n<%s|View post on Bluesky ↗>", bskyMessage.Commit.Record.Text, bskyMessage.toURL(&profile.Handle)),
Text: fmt.Sprintf("%s\n<%s|View post on Bluesky ↗>", messageText, bskyMessage.toURL(&profile.Handle)),
ImageUrl: imageURL,
Footer: "Posted",
Ts: strconv.FormatInt(postTime.Unix(), 10),
Expand Down
29 changes: 26 additions & 3 deletions cmd/bsky-webhook/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,10 @@ type BskyCommit struct {
}

type BskyRecord struct {
Text string `json:"text"`
Embed BskyEmbed `json:"embed"`
CreatedAtString string `json:"createdAt"`
Text string `json:"text"`
Embed BskyEmbed `json:"embed"`
CreatedAtString string `json:"createdAt"`
Copy link
Member

Choose a reason for hiding this comment

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

Maybe a comment noting the expected format? (Is this RFC3339 or something?)

Facets []BskyFacet `json:"facets"`
}

type BskyEmbed struct {
Expand All @@ -50,6 +51,28 @@ type BskyImageRef struct {
Link string `json:"$link"`
}

type BskyFacet struct {
Features []BskyFacetFeatures `json:"features"`
Index BskyFacetIndex `json:"index"`
}

type BskyFacetFeatures struct {
Type string `json:"$type"`
Uri string `json:"uri"`
Copy link
Member

Choose a reason for hiding this comment

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

By convention these should be URI and DID.

Did string `json:"did"`
Copy link
Member

@creachadair creachadair Jan 3, 2025

Choose a reason for hiding this comment

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

Do we need the names explicitly here? It appears we only use these for decoding, and the JSON decoder does a case-insensitive match anyway. (We do need $type because of the spelling difference, but I mean the others)

Tag string `json:"tag"`
}

type BskyFacetIndex struct {
ByteEnd int `json:"byteEnd"`
ByteStart int `json:"byteStart"`
}

type BskyTextFragment struct {
Text string
Features []BskyFacetFeatures
}

type SlackAttachment struct {
AuthorName string `json:"author_name"`
AuthorIcon string `json:"author_icon"`
Expand Down
Loading