Skip to content

Conversation

@iglosiggio
Copy link
Contributor

This way we can quickly reference and navigate to a given package tag.

Made during the pharo sprint with @matijakljajic

Co-Authored-By: Matija Kljajic <matijakljajic173@gmail.com>
@iglosiggio iglosiggio force-pushed the browse-to-package-tag branch from 2df4eb0 to 1e1784d Compare January 31, 2025 16:38
@Ducasse
Copy link
Contributor

Ducasse commented Feb 2, 2025

Thanks I will have a look.
Would be good to have tests and getXXX is not that idiomatic of Pharo.
I will have a look.

@Ducasse
Copy link
Contributor

Ducasse commented Feb 2, 2025

I know that the existing code is using this getX stuff.

@iglosiggio
Copy link
Contributor Author

Yes, we continued the code following what was already there. Now it's probably time to improve it.

OTOH there's no test on this PR and we are adding a new feature. That's not right :P

My rewrite would be:

  • Remove the object mutation from the getX methods
  • Add a field containing the list of supported "entity providers" (an entity provider is something that respondsTo #value: and expects the token list as argument)
  • Make the initial value of this list something like: { [ :tokens | self maybeGetClass ]. [ :tokens | self maybeGetPackage ]. [ :tokens | self maybeGetTag ] } (if we put a bit more work we can migrate the whole computeEntity method to a series of calls like that)
  • Then the implementation of computeEntity is just entity := providers detect: [ :provider | provider value: tokens ].

@Ducasse
Copy link
Contributor

Ducasse commented Aug 2, 2025

I checked and it looks ok for now.
So I will integrate it.

@Ducasse Ducasse merged commit dc1ec43 into pillar-markup:dev Aug 2, 2025
2 checks passed
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