Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

Post integration of the new implementation #12

Open
Ducasse opened this issue Oct 6, 2021 · 6 comments
Open

Post integration of the new implementation #12

Ducasse opened this issue Oct 6, 2021 · 6 comments

Comments

@Ducasse
Copy link
Contributor

Ducasse commented Oct 6, 2021

Kasper I'm fixing several points:

  • I added a dev branch and we should do PR on this branch so that after once this is ok we can merge into master (= into pharo).
  • We should not use Smalltalk ui icons but self iconNamed:

I'm trying to fix the class comments but I cannot.

  • how do we decide to internalize the file? because I do not get when I decide it.

  • I have no idea what is openOnCustomHelp: aDescription there is no comment and no sender.

  • openPharoRepo just breaks on me.

  • open
    "Open a HelpBrowser on the internalized docs."

    ^ self error: 'We need to define a default starting point'

I have no idea what is a default starting point and how to do it so the error does not help the user at all.

  • the following raises a DNU:
openBrowser
	<script>
	StHelpBrowserPresenter openTopic: self githubTopics 

after I press reset, it is working.
Before I was using StHelpBrowser open ....

@Ducasse
Copy link
Contributor Author

Ducasse commented Oct 6, 2021

To me this method

openBrowser
	<script>
	StHelpBrowserPresenter openTopic: self githubTopics 

should be moved to StHelpBrowserPresenter because users do not care about the builder.
It should probably named openPharoDocFromGit or something like that.

@Ducasse
Copy link
Contributor Author

Ducasse commented Oct 6, 2021

  • I could not get how to use StTopicFromFileFolderBuilder to build from local files.

@Ducasse
Copy link
Contributor Author

Ducasse commented Oct 6, 2021

I saw that it is used in `StHelpBrowserPresenter class>> #openOnHelpFolder: so I added a class comment.

@Ducasse
Copy link
Contributor Author

Ducasse commented Oct 6, 2021

  • I could not understand why the visitor is holding an instance of itself.
StTopicBuilderVisitor >> helpTopicVisitor
	
	helpTopicVisitor ifNil: [ helpTopicVisitor := StTopicBuilderVisitor ].
	^ helpTopicVisitor

@Ducasse
Copy link
Contributor Author

Ducasse commented Oct 6, 2021

This looks fishy to me.

@Ducasse
Copy link
Contributor Author

Ducasse commented Oct 6, 2021

I removed it and it works.

Ducasse added a commit that referenced this issue Oct 6, 2021
- Remove unused code
- Check comments.
- Recategorised some methods.
See #12 for next actions.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant