Regression with alert #111

Closed
Dassadar opened this Issue Apr 9, 2015 · 14 comments

Comments

Projects
None yet
5 participants
@Dassadar

Dassadar commented Apr 9, 2015

Hello,

On latest version of Shoes, alert no longer works the same.
If we have:
a = "test"
Then:

  • in previous version, alert a worked
  • in current version, alert a fails, and only possibility is alert "#{a}"

Regards,
Dassadar

@ccoupe

This comment has been minimized.

Show comment
Hide comment
@ccoupe

ccoupe Apr 9, 2015

Contributor
Shoes.app do
   a = "test"
   alert a
end

works for me.

Have you looked in the Shoes console for an error message? type alt-/

Contributor

ccoupe commented Apr 9, 2015

Shoes.app do
   a = "test"
   alert a
end

works for me.

Have you looked in the Shoes console for an error message? type alt-/

@Dassadar

This comment has been minimized.

Show comment
Hide comment
@Dassadar

Dassadar Apr 9, 2015

Sorry, bad example.
This one doesn't work and worked before:

Shoes.app do
a = 3
alert a
end

2015-04-09 8:15 GMT+02:00 Cecil Coupe notifications@github.com:

Shoes.app do
a = "test"
alert aend

works for me.

Have you looked in the Shoes console for an error message? type alt-/


Reply to this email directly or view it on GitHub
#111 (comment).

Dassadar commented Apr 9, 2015

Sorry, bad example.
This one doesn't work and worked before:

Shoes.app do
a = 3
alert a
end

2015-04-09 8:15 GMT+02:00 Cecil Coupe notifications@github.com:

Shoes.app do
a = "test"
alert aend

works for me.

Have you looked in the Shoes console for an error message? type alt-/


Reply to this email directly or view it on GitHub
#111 (comment).

@ProvGANG

This comment has been minimized.

Show comment
Hide comment
@ProvGANG

ProvGANG Apr 9, 2015

you have to put it like this alert a.to_s and it'll work, numbers are
considered different "type" of data than strings in ruby and programming in
general, .to_s converts your number from 3 to "3" which then can be written
withing an alert widget

2015-04-09 20:42 GMT+02:00 Dassadar notifications@github.com:

Sorry, bad example.
This one doesn't work and worked before:

Shoes.app do
a = 3
alert a
end

2015-04-09 8:15 GMT+02:00 Cecil Coupe notifications@github.com:

Shoes.app do
a = "test"
alert aend

works for me.

Have you looked in the Shoes console for an error message? type alt-/


Reply to this email directly or view it on GitHub
#111 (comment).


Reply to this email directly or view it on GitHub
#111 (comment).

Cheers!

ProvGANG commented Apr 9, 2015

you have to put it like this alert a.to_s and it'll work, numbers are
considered different "type" of data than strings in ruby and programming in
general, .to_s converts your number from 3 to "3" which then can be written
withing an alert widget

2015-04-09 20:42 GMT+02:00 Dassadar notifications@github.com:

Sorry, bad example.
This one doesn't work and worked before:

Shoes.app do
a = 3
alert a
end

2015-04-09 8:15 GMT+02:00 Cecil Coupe notifications@github.com:

Shoes.app do
a = "test"
alert aend

works for me.

Have you looked in the Shoes console for an error message? type alt-/


Reply to this email directly or view it on GitHub
#111 (comment).


Reply to this email directly or view it on GitHub
#111 (comment).

Cheers!

@Dassadar

This comment has been minimized.

Show comment
Hide comment
@Dassadar

Dassadar Apr 9, 2015

Yes. But precisely, my point was that it was possible in previous Shoes
version!

2015-04-09 20:52 GMT+02:00 Luka Povreslo notifications@github.com:

you have to put it like this alert a.to_s and it'll work, numbers are
considered different "type" of data than strings in ruby and programming in
general, .to_s converts your number from 3 to "3" which then can be written
withing an alert widget

2015-04-09 20:42 GMT+02:00 Dassadar notifications@github.com:

Sorry, bad example.
This one doesn't work and worked before:

Shoes.app do
a = 3
alert a
end

2015-04-09 8:15 GMT+02:00 Cecil Coupe notifications@github.com:

Shoes.app do
a = "test"
alert aend

works for me.

Have you looked in the Shoes console for an error message? type alt-/


Reply to this email directly or view it on GitHub
#111 (comment).


Reply to this email directly or view it on GitHub
#111 (comment).

Cheers!


Reply to this email directly or view it on GitHub
#111 (comment).

Dassadar commented Apr 9, 2015

Yes. But precisely, my point was that it was possible in previous Shoes
version!

2015-04-09 20:52 GMT+02:00 Luka Povreslo notifications@github.com:

you have to put it like this alert a.to_s and it'll work, numbers are
considered different "type" of data than strings in ruby and programming in
general, .to_s converts your number from 3 to "3" which then can be written
withing an alert widget

2015-04-09 20:42 GMT+02:00 Dassadar notifications@github.com:

Sorry, bad example.
This one doesn't work and worked before:

Shoes.app do
a = 3
alert a
end

2015-04-09 8:15 GMT+02:00 Cecil Coupe notifications@github.com:

Shoes.app do
a = "test"
alert aend

works for me.

Have you looked in the Shoes console for an error message? type alt-/


Reply to this email directly or view it on GitHub
#111 (comment).


Reply to this email directly or view it on GitHub
#111 (comment).

Cheers!


Reply to this email directly or view it on GitHub
#111 (comment).

@passenger94

This comment has been minimized.

Show comment
Hide comment
@passenger94

passenger94 Apr 9, 2015

Contributor

right ...
before: msg = shoes_native_to_s(msg); --> automatic conversion (calling :to_s on msg)
now: RSTRING_PTR(args.a[0]); --> needs a string as input
what is the way to go, ancient behavior ?, new one ?

Contributor

passenger94 commented Apr 9, 2015

right ...
before: msg = shoes_native_to_s(msg); --> automatic conversion (calling :to_s on msg)
now: RSTRING_PTR(args.a[0]); --> needs a string as input
what is the way to go, ancient behavior ?, new one ?

@Dassadar

This comment has been minimized.

Show comment
Hide comment
@Dassadar

Dassadar Apr 9, 2015

I think this breaks backward compatibility... so I'd go for the automatic
conversion.

2015-04-09 21:07 GMT+02:00 passenger94 notifications@github.com:

right ...

before: msg = shoes_native_to_s(msg); --> automatic conversion (calling
:to_s on msg)
now: RSTRING_PTR(args.a[0]); --> needs a string as input
what is the way to go, ancient behavior ?, new one ?


Reply to this email directly or view it on GitHub
#111 (comment).

Dassadar commented Apr 9, 2015

I think this breaks backward compatibility... so I'd go for the automatic
conversion.

2015-04-09 21:07 GMT+02:00 passenger94 notifications@github.com:

right ...

before: msg = shoes_native_to_s(msg); --> automatic conversion (calling
:to_s on msg)
now: RSTRING_PTR(args.a[0]); --> needs a string as input
what is the way to go, ancient behavior ?, new one ?


Reply to this email directly or view it on GitHub
#111 (comment).

@ccoupe

This comment has been minimized.

Show comment
Hide comment
@ccoupe

ccoupe Apr 9, 2015

Contributor

I'm not a fan of the auto conversion to string. Just one opinion.

Contributor

ccoupe commented Apr 9, 2015

I'm not a fan of the auto conversion to string. Just one opinion.

@Dassadar

This comment has been minimized.

Show comment
Hide comment
@Dassadar

Dassadar Apr 9, 2015

But take the point of view of a guy who would have developed a massive app
with plenty of uses of unconverted alert?
Being in the software industry, for any software, my point of view is that
we cannot break the caller's program just by making him migrate to the most
recent version!

2015-04-09 21:13 GMT+02:00 Cecil Coupe notifications@github.com:

I'm not a fan of the auto conversion to string. Just one opinion.


Reply to this email directly or view it on GitHub
#111 (comment).

Dassadar commented Apr 9, 2015

But take the point of view of a guy who would have developed a massive app
with plenty of uses of unconverted alert?
Being in the software industry, for any software, my point of view is that
we cannot break the caller's program just by making him migrate to the most
recent version!

2015-04-09 21:13 GMT+02:00 Cecil Coupe notifications@github.com:

I'm not a fan of the auto conversion to string. Just one opinion.


Reply to this email directly or view it on GitHub
#111 (comment).

@BackOrder

This comment has been minimized.

Show comment
Hide comment
@BackOrder

BackOrder Apr 9, 2015

Collaborator

Auto conversion is kinda used everywhere in Shoes, isn't it? Wouldn't it make sense to keep it this way? And @ccoupe always reminded me how important backward compatibility is.

Collaborator

BackOrder commented Apr 9, 2015

Auto conversion is kinda used everywhere in Shoes, isn't it? Wouldn't it make sense to keep it this way? And @ccoupe always reminded me how important backward compatibility is.

@ccoupe

This comment has been minimized.

Show comment
Hide comment
@ccoupe

ccoupe Apr 9, 2015

Contributor

I also remember a bug report where someone passed an array in and the auto conversion didn't do the expected thing. ;^). Practically speaking, unless you are lobbying for an emergency bug release and I'm kind of busy for the next few days, you're stuck converting your alerts.

Contributor

ccoupe commented Apr 9, 2015

I also remember a bug report where someone passed an array in and the auto conversion didn't do the expected thing. ;^). Practically speaking, unless you are lobbying for an emergency bug release and I'm kind of busy for the next few days, you're stuck converting your alerts.

@Dassadar

This comment has been minimized.

Show comment
Hide comment
@Dassadar

Dassadar Apr 9, 2015

ok ;-)

2015-04-09 21:33 UTC+02:00, Cecil Coupe notifications@github.com:

I also remember a bug report where someone passed an array in and the auto
conversion didn't do the expected thing. ;^). Practically speaking, unless
you are lobbying for an emergency bug release and I'm kind of busy for the
next few days, you're stuck converting your alerts.


Reply to this email directly or view it on GitHub:
#111 (comment)

Dassadar commented Apr 9, 2015

ok ;-)

2015-04-09 21:33 UTC+02:00, Cecil Coupe notifications@github.com:

I also remember a bug report where someone passed an array in and the auto
conversion didn't do the expected thing. ;^). Practically speaking, unless
you are lobbying for an emergency bug release and I'm kind of busy for the
next few days, you're stuck converting your alerts.


Reply to this email directly or view it on GitHub:
#111 (comment)

@ProvGANG

This comment has been minimized.

Show comment
Hide comment
@ProvGANG

ProvGANG Apr 10, 2015

I'm not sure if automatic conversion to string is always a good idea, let's give an example where you want to sum two numbers, let's say 1 + 2, and the result won't be 3 but "12". Of course those numbers can be summed before invoking alert on it, but to me it seems like quite an unusual behavior.

I'm not sure if automatic conversion to string is always a good idea, let's give an example where you want to sum two numbers, let's say 1 + 2, and the result won't be 3 but "12". Of course those numbers can be summed before invoking alert on it, but to me it seems like quite an unusual behavior.

@ccoupe

This comment has been minimized.

Show comment
Hide comment
@ccoupe

ccoupe Apr 10, 2015

Contributor

I understand both sides. The programmer should pass a string where a string is asked for. It's an error otherwise. But, if the arg is the 'wrong' type then what should alert() do - Nothing ? Throw an exception ? Print what it can? The first two options are unfriendly, particular for an alert that presumably is there for reporting an important event or state. The second arg (title) is auto converted. From the bugs/bug059.rb file

alert "Crash?", title: {:a => "hey"}

will work without complaining even though we know it's an error.

Contributor

ccoupe commented Apr 10, 2015

I understand both sides. The programmer should pass a string where a string is asked for. It's an error otherwise. But, if the arg is the 'wrong' type then what should alert() do - Nothing ? Throw an exception ? Print what it can? The first two options are unfriendly, particular for an alert that presumably is there for reporting an important event or state. The second arg (title) is auto converted. From the bugs/bug059.rb file

alert "Crash?", title: {:a => "hey"}

will work without complaining even though we know it's an error.

@ccoupe

This comment has been minimized.

Show comment
Hide comment
@ccoupe

ccoupe Aug 8, 2015

Contributor

Shoes 3.2.24 restores the old behaviour.

Contributor

ccoupe commented Aug 8, 2015

Shoes 3.2.24 restores the old behaviour.

@ccoupe ccoupe closed this Aug 12, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment