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

Use better show instances for notes & rationals #857

Merged
merged 1 commit into from
Nov 6, 2021

Conversation

matthewkaney
Copy link
Collaborator

In the discussion from #807 it came up that the Show instance for notes is a bit confusing, because it exposes the structure of the Note type. I added a new show function that prints the numeric value followed by the note name. Here's what it looks like in the context of a pattern:

> n "c d6 20.75 -30"
(0>¼)|n: 0.0n (c5)
(¼>½)|n: 14.0n (d6)
(½>¾)|n: 20.75n (a6)
(¾>1)|n: -30.0n (fs2)

I also updated the Show instance for patterns to use the prettier rational representation (used by time spans) for rational values.

@matthewkaney
Copy link
Collaborator Author

Hmm... it looks like the stack run is failing to install the happy package, probably because the version it's installing (1.21.0) seems to be deprecated.

I don't really know anything about stack, or this specific build job, so if folks have advice, that would be appreciated!

@yaxu
Copy link
Member

yaxu commented Oct 25, 2021

Thanks @mindofmatthew, this looks great. I wonder if the note name shouldn't be shown for non-integers?

I'm not sure what's going on with happy, it seems strange to have the latest version deprecated. Maybe it's best wait and see if that sorts itself out..

@yaxu
Copy link
Member

yaxu commented Nov 1, 2021

Aside from the stack issue (which isn't particular to this PR), is this ready for merge?

@matthewkaney
Copy link
Collaborator Author

Yes, unless you (or anyone else) wants a different display for non-integer notes. I think the most accurate representation would be something like 2.7n (ds5 - 30 cents) but that's maybe too long? Happy to drop the pitch names from non-integers or add a symbol like +d5 or <ds5 to indicate when a number is in between pitches.

@yaxu yaxu merged commit 4c5c252 into tidalcycles:main Nov 6, 2021
@yaxu
Copy link
Member

yaxu commented Nov 6, 2021

Lets go with this for now, thanks!

@matthewkaney matthewkaney deleted the note-ui branch November 10, 2022 22:05
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.

2 participants