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

File Browser > Changes - FileList DNU #readStream #3302

Closed
bencoman opened this issue May 6, 2019 · 5 comments
Closed

File Browser > Changes - FileList DNU #readStream #3302

bencoman opened this issue May 6, 2019 · 5 comments

Comments

@bencoman
Copy link
Contributor

bencoman commented May 6, 2019

  1. Tools > File Browser
  2. Select an ST file
  3. Click Changes

==> Instance of FileList did not understand #readStream

Fix is to change that to #readOnlyStream as hinted at... https://pharo.fogbugz.com/f/cases/19859

ExternalChangesBrowser>>serviceBrowseCSOrSTFile
	"Answer a service for opening a changelist browser on a file"
	^ (SimpleServiceEntry 
		provider: self 
		label: 'Changelist browser'
		selector: #openOnStream:
		description: 'Open a changelist tool on this file'
		buttonLabel: 'Changes')
-		argumentGetter: [ :stream | stream readStream ]
+		argumentGetter: [ :stream | stream readStreamOnly ]

And same for ExternalBrowser>>serviceBrowseCode

(P.S. Its strange that case indicates it was integrated but it seems to be missing.)

Can this also be backported for the next Pharo 7 minor release. It would be pretty jarring for a newcomer browsing around features.

@bencoman
Copy link
Contributor Author

bencoman commented May 6, 2019

I was doing up a PR but find the FileList>>readOnlyStream is using deprecated FileStream

FileList >> readOnlyStream
	"Answer a read-only stream on the selected file. For the various stream-reading services."
	^ FileStream readOnlyFileNamed: self reference fullName

so another solution is required for Pharo 8. I'm not familiar with what the planned replacement should be so I'll have to leave it to others.

bencoman added a commit to bencoman/pharo that referenced this issue May 6, 2019
@svenvc
Copy link
Contributor

svenvc commented May 6, 2019

But the reference inside FileList is already a FileReference, so the fix is to just ask the reference for its readStream

estebanlm added a commit that referenced this issue May 21, 2019
@estebanlm
Copy link
Member

@bencoman can I close this issue now?

@bencoman
Copy link
Contributor Author

Thx @estebanlm. I've confirmed its fixed in build...
Pharo7.0.0-rc1.build.1436.sha.e281eb4.arch.32bit

Side thought... only if its simple to do... it could be good for the "development versions" shown in Launcher to follow the releases, so this build might have been named...
Pharo7.0.4-alpha.build.1436.sha.e281eb4.arch.32bit

@bencoman
Copy link
Contributor Author

@estebanlm So I was slow to notice that its been fixed in Pharo7.0.3.
Thats cool, although the idealist in me would have liked the patch version bumped.

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

No branches or pull requests

3 participants