code #418
code #418
Conversation
| # frozen_string_literal: true | ||
|
|
||
| module Shoes | ||
| class Codes < Shoes::Drawable |
There was a problem hiding this comment.
hm when i use code it giving me superclass mismatch idk if some other gem we using has it?
There was a problem hiding this comment.
That would be weird, because it's under Shoes.
There was a problem hiding this comment.
I don't get the superclass mismatch when I check out your branch. Maybe try a "git status" and see if you have any un-checked-in changes that might cause the problem?
There was a problem hiding this comment.
is that why this PR isn't using code as a widget (e.g.here https://github.com/scarpe-team/scarpe/blob/main/docs/static/manual.md)
There was a problem hiding this comment.
It looks like we have a code TextDrawable already, called "code". So yeah, this is a bit odd - one called "codes", but also a regular Drawable not a TextDrawable.
So if you declared it as Code, not Codes, you'd get the superclass error because Code is a child of Shoes::TextDrawable.
There was a problem hiding this comment.
this is what codes does
There was a problem hiding this comment.
should i remove this? i dont think we are using code anywhwere? in examples?
Should we remove the code text drawable, and make this PR code for this part of the spec?
There was a problem hiding this comment.
There was a problem hiding this comment.
@gintama91 btw this looks super cool :)
There was a problem hiding this comment.
It really does look awesome :-)
It's true that we have no legacy examples using code(), only more recent ones. In Shoes3, looking at their source, it looks like code (grep for cCode) is a text type, so you can use it like em() inside a para(), and it's basically just monospaced like typewriter font in HTML.
If we accept this, it should definitely be renamed to replace the other code element, not add ones named codes.
|
I'd love to have this merged if possible, the code looks great, as "code" |
|
If we merge this as "code" we'll want to figure out how to update the Shoes manual, etc. Because old Shoes already has something called code and it doesn't work like this. |
| @@ -0,0 +1,3 @@ | |||
| Shoes.app do | |||
| codes "Shoes.app do\n para 'Hello, world!'\nend" | |||
There was a problem hiding this comment.
@noahgibbs can you explain to me the differences?
Our examples don't have much use of this historically. And the manual says it is a text fragment
There was a problem hiding this comment.
In fact the only examples are ours. So, I'm possibly willing to go out on a limb for this feature? WDYT?
There was a problem hiding this comment.
The main difference is that one is a text fragment, like em() or strong() or sup(). It goes inside a para. So you'd do something like this:
para "Yup, this is ", strong(em("totally")), code(" made of code"), "!"
Pavan's codes() here is a Drawable like para. It does formatting but doesn't really support other formatting inside it (unless we don't escape the HTML, but Shoes doesn't normally let you just put random HTML in places and have it format correctly.)
There was a problem hiding this comment.
Also, given that we now have the ability to add Scarpe-specific extensions, I think it should be one of those.
|
If this gets approved , i'll add syntax highlighting ig, dk how exactly but should be easy. |
Could we add it as a new optional feature and call it "kode"? I'd like to explore this approach but appreciate that it is not canonical shoes. But in 2023 it's an excellent addition. It would be v handy for building out a native shoes school I think! |
|
Yup, as long as we don't use the same name it's fine. |
Description
Image(if needed, helps for a faster review)
Checklist