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

Remove BLCursor in favor of Cursor #400

Merged
merged 2 commits into from
Jan 16, 2024
Merged

Conversation

jecisc
Copy link
Member

@jecisc jecisc commented Jan 15, 2024

This change removes the while hierarchy of BLCursor to use the Cursor class instead.

All subclasses were returning a form from the Cursor class and it was unused.

This should be more efficient since it will not recreate a form all the time but use a saved instance.

@jecisc
Copy link
Member Author

jecisc commented Jan 16, 2024

I am not sure how the change broke the tests

@plantec
Copy link
Collaborator

plantec commented Jan 16, 2024

thanks !

@tinchodias
Copy link
Collaborator

Hi @jecisc, thanks. This change reminds me to another around keyboard: there was Bl* that duplicated/wrapped the ones in Pharo. I agree if there is no need that we can directly use the Pharo ones. The class Cursor belongs to a Graphics* package and we might want to avoid the dependency but for now there dependency was there, so... you are not adding a dependency. I think it's okay.

In P11 the 2 failures are my fault. I will fix now. In P12, It's missing Time millisecondsToRun: I think @Ducasse removed it? I will check a replacement or removal.

@tinchodias
Copy link
Collaborator

Ah... who sends millisecondsToRun: and errors is smalltalk-ci, not Bloc:

Instance of Time class did not understand #millisecondsToRun:
Time class(Object)>>doesNotUnderstand: #millisecondsToRun:
SmalltalkCI class>>timeToRun:
SmalltalkCIPharo12 class(SmalltalkCI class)>>fold:on:block:
SmalltalkCIPharo12 class(SmalltalkCI class)>>fold:block:
SmalltalkCIPharo12 class(SmalltalkCI class)>>stage:id:block:
SmalltalkCIPharo12(SmalltalkCI)>>reportImageInfo
SmalltalkCIPharo12(SmalltalkCI)>>basicLoad

@jecisc
Copy link
Member Author

jecisc commented Jan 16, 2024

The Smalltalk CI one is known and is been fixed IIRC.

I was not sure about the P11 ones :) Thanks

@Ducasse
Copy link
Contributor

Ducasse commented Jan 16, 2024

Martin we will reintroduce the Time milliseconds: and deprecate it.

@tinchodias
Copy link
Collaborator

So, let's merge it

@tinchodias tinchodias merged commit 24657e0 into pharo-graphics:dev-1.0 Jan 16, 2024
0 of 6 checks passed
@plantec
Copy link
Collaborator

plantec commented Jan 16, 2024

thanks.
now I have to fix Toplo :)

@Ducasse
Copy link
Contributor

Ducasse commented Jan 16, 2024

Excellent.

@jecisc
Copy link
Member Author

jecisc commented Jan 17, 2024

thanks. now I have to fix Toplo :)

I did not think about Toplo sorry.

I can take a look this week if you did not do it already

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.

4 participants