-
Notifications
You must be signed in to change notification settings - Fork 1
Added spotify playback and other stuff #83
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,15 +24,16 @@ | |
|
|
||
| ### Spotify | ||
|
|
||
| - `add to queue` and `play now` features (need write access) | ||
| - Allow users to add songs to playlists | ||
|
|
||
|
|
||
| ## General capabilities | ||
|
|
||
| - Restricted write permissions | ||
| - Restricted write permissions (Spotify REMOTE actions ship ahead of this; the | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. comment quality: this parenthetical is PR description material, the roadmap item should stay as it was |
||
| shell's credential-worker message is the seam to wrap) | ||
| - Enable drop-in self-coded plugins | ||
|
|
||
|
|
||
| ## Patches | ||
|
|
||
| - Show update progress | ||
| - Show update progress | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please don't change the architecture requirements |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,11 @@ | ||
| # Architecture (Written by John Clanker) | ||
|
|
||
| smorg is a keyboard-driven terminal dashboard: each connected integration is a | ||
| tab, nothing is enabled by default, and the app is read-plus-safe-actions — it | ||
| shows what's on your plate and opens things, it never writes to a service. | ||
| tab, nothing is enabled by default, and the app is read-plus-safe-actions by | ||
| default — it shows what's on your plate and opens things. Writes to a service | ||
| are exceptional: today only Spotify's play/queue actions (`ActionClass.REMOTE`) | ||
| mutate remote state, via a shell-mediated credential worker that keeps panels | ||
| off the network. A general restricted-write permission model is still ahead. | ||
|
|
||
| This document explains the load-bearing decisions. How to *add* an integration | ||
| is covered in [CONTRIBUTING.md](../CONTRIBUTING.md). | ||
|
|
@@ -67,13 +70,11 @@ look like one: | |
|
|
||
| | | MCP transport | REST transport | | ||
| | ---------------- | ------------- | -------------- | | ||
| | **OAuth** | Linear | — | | ||
| | **OAuth** | Linear | Spotify | | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. keep this row: Spotify was already OAuth + REST on main while the doc still called it a roadmap candidate. It's the prose above that has to go back, not this |
||
| | **Pasted token** | — | GitHub | | ||
|
|
||
| The empty corners are circumstance, not design. OAuth + REST is where any | ||
| classic OAuth provider with no token alternative lands (Spotify is the | ||
| roadmap's first candidate); token + MCP would be an MCP server reached with a | ||
| static bearer token. | ||
| The empty corners are circumstance, not design. Token + MCP would be an MCP | ||
| server reached with a static bearer token. | ||
|
|
||
| ### The auth axis: OAuth where it is cheap, a pasted token where it is not | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,6 +15,7 @@ classifiers = [ | |
| dependencies = [ | ||
| "httpx>=0.28", | ||
| "keyring>=25.0", | ||
| "pillow>=12.3.0", | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. dependency: 12.3.0 is the version that happened to be installed, not the oldest that works. Also a hard dependency for one integration's cosmetic feature, worth a thought |
||
| "pygithub>=2.5", | ||
| "requests>=2.32", | ||
| "textual>=1.0", | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,7 +24,8 @@ class ActionClass(StrEnum): | |
|
|
||
| LOCAL -> our own state | ||
| LAUNCH -> browser or clipboard | ||
| REMOTE -> API (not implemented yet) | ||
| REMOTE -> API mutation (shell runs it via CredentialWorkRequested; a general | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Comments talk about the future. Docstrings in contract.py and shell/panel.py describe plans ("Plan B can wrap this"). Comments should say what the code does now. |
||
| write-permission gate is still ahead) | ||
| """ | ||
|
|
||
| LOCAL = "local" | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,63 @@ | ||
| """Turn album artwork bytes into colored terminal ASCII.""" | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| from io import BytesIO | ||
|
|
||
| from PIL import Image, ImageEnhance | ||
| from rich.color import Color | ||
| from rich.style import Style | ||
| from rich.text import Text | ||
|
|
||
| # Characters go from dark -> bright. | ||
| CHARS = ' .`^",:;Il!i~+_-?][}{1)(|\\/~tfjrxnuvczXYUJCLQ0OZmwqpdbkhao*#MW&8%B@$' | ||
| # Terminal cells are roughly twice as tall as they are wide. | ||
| CELL_ASPECT = 0.5 | ||
|
|
||
|
|
||
| def image_to_ascii( | ||
| data: bytes, | ||
| width: int, | ||
| *, | ||
| contrast: float = 1.0, | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. code quality: contrast and brightness are never passed by the app, leftovers from the scratch CLI |
||
| brightness: float = 1.0, | ||
| ) -> Text | None: | ||
| """Colored ASCII art of `data`, `width` cells wide, or None if the bytes are not an image.""" | ||
| if width < 1 or not data: | ||
| return None | ||
| try: | ||
| image = Image.open(BytesIO(data)).convert("RGB") | ||
| except (OSError, ValueError): | ||
| return None | ||
| if contrast != 1.0: | ||
| image = ImageEnhance.Contrast(image).enhance(contrast) | ||
| if brightness != 1.0: | ||
| image = ImageEnhance.Brightness(image).enhance(brightness) | ||
| original_width, original_height = image.size | ||
| if original_width < 1 or original_height < 1: | ||
| return None | ||
| height = max(1, int((original_height / original_width) * width * CELL_ASPECT)) | ||
| image = image.resize((width, height), Image.Resampling.LANCZOS) | ||
| pixels = image.load() | ||
| if pixels is None: | ||
| return None | ||
| max_char_index = len(CHARS) - 1 | ||
| lines: list[Text] = [] | ||
| for y in range(height): | ||
| line = Text() | ||
| for x in range(width): | ||
| pixel = pixels[x, y] | ||
| if not isinstance(pixel, tuple): | ||
| continue | ||
| red = int(pixel[0]) | ||
| green = int(pixel[1]) | ||
| blue = int(pixel[2]) | ||
| luminance = 0.2126 * red + 0.7152 * green + 0.0722 * blue | ||
| char_index = int((luminance / 255) * max_char_index) | ||
| char = CHARS[char_index] | ||
| color = Color.from_rgb(red, green, blue) | ||
| line.append(char, style=Style(color=color)) | ||
| lines.append(line) | ||
| body = Text("\n").join(lines) | ||
| body.no_wrap = True | ||
| return body | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| """Spotify's declaration; connects with OAuth against an app the user creates themselves, and reads | ||
| the REST API. | ||
| """Spotify's declaration; connects with OAuth against an app the user creates themselves, and talks | ||
| to the REST API (read plus play/queue writes). | ||
| """ | ||
|
|
||
| from __future__ import annotations | ||
|
|
@@ -28,6 +28,7 @@ | |
| "user-read-currently-playing", | ||
| "user-read-playback-state", | ||
| "user-read-recently-played", | ||
| "user-modify-playback-state", | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. token scope: adding a scope here does nothing for anyone already connected. Their keychain token was granted with the three read scopes, refresh keeps that grant, and nothing compares the stored credentials.scope to this list. So after upgrading, reads work and every new action gets a 403, which _request turns into "add the account to the app's allowlist". Wrong fix, the right one is smorg connect spotify, and nothing tells the user that. What I want: compare granted scopes to the manifest's before running remote work and say "reconnect to grant user-modify-playback-state" up front, the way AuthExpired already gets its reconnect hint. That check is also the hook the write-permission gate needs later. See the comment in app.py for where it goes |
||
| ), | ||
| ) | ||
|
|
||
|
|
@@ -42,6 +43,26 @@ | |
| Action(id="open", label="Open in Spotify", key="o", action_class=ActionClass.LAUNCH), | ||
| Action(id="play_now", label="Play now", key="p", action_class=ActionClass.REMOTE), | ||
| Action(id="add_to_queue", label="Add to queue", key="a", action_class=ActionClass.REMOTE), | ||
| Action(id="toggle_shuffle", label="Shuffle", key="s", action_class=ActionClass.REMOTE), | ||
| Action(id="cycle_repeat", label="Repeat", key="e", action_class=ActionClass.REMOTE), | ||
| Action( | ||
| id="toggle_playback", | ||
| label="Play/pause", | ||
| key="space", | ||
| action_class=ActionClass.REMOTE, | ||
| ), | ||
| Action( | ||
| id="skip_previous", | ||
| label="Previous", | ||
| key="comma", | ||
| action_class=ActionClass.REMOTE, | ||
| ), | ||
| Action( | ||
| id="skip_next", | ||
| label="Next", | ||
| key="full_stop", | ||
| action_class=ActionClass.REMOTE, | ||
| ), | ||
| ), | ||
| ) | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
comment quality: the parenthetical is a plan, not documentation. And if the mechanism changes per the architecture comments this bullet changes with it