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 Dialog: Add dynamic preview #682

Merged
merged 13 commits into from
Feb 8, 2024

Conversation

hernanmd
Copy link
Member

@hernanmd hernanmd commented Feb 6, 2024

This PR adds the following features to File Dialog support:

  • Add a method to find previewers providing its extension or a FileReference.
  • Add GIF previewer
  • Add previewer tests.

@hernanmd hernanmd self-assigned this Feb 6, 2024
@hernanmd hernanmd added the enhancement New feature or request label Feb 6, 2024
Hernán Morales Durand added 2 commits February 7, 2024 19:54
Fix typos.
Adjust window size and position.
@Ducasse
Copy link
Contributor

Ducasse commented Feb 7, 2024

@hernanmd you have

✗ #testObsoleteClasses (837ms)
TestFailure: Obsolete classes remaining: an Array(AnObsoleteStFileBrowserContentPreviewer)
ReleaseTest(TestAsserter)>>assert:description:resumable:
ReleaseTest(TestAsserter)>>assert:description:
ReleaseTest>>testObsoleteClasses ...assert: obsoleteClasses isEmpty
		description: [
			String streamContents: [ :s|
				s
					nextPutAll: 'Obsolete classes remaining: ';
					print: obsoleteClasses ]]
ReleaseTest(TestCase)>>performTest

@hernanmd
Copy link
Member Author

hernanmd commented Feb 8, 2024

Hi @jecisc, I tried to reproduce locally the failing Pharo-Integration tests but even after fixing a Sindarin loading error, the tests (testNoUnusedClassVariablesLeftand testObsoleteClasses) are green. This is how I load in a new Pharo 12 image:

Metacello new
	baseline: 'NewTools';
	repository: 'github://hernanmd/NewTools:add_dynamic_preview/src';
	load.

I also tried running through smalltalkCI but it was worst as Spec changes were missing:

bin/smalltalkci --headful -s Pharo64-12 ../git-repositories/NewTools/.smalltalk.ston

Do you know what else to try?

@hernanmd
Copy link
Member Author

hernanmd commented Feb 8, 2024

Hi @jecisc, I tried to reproduce locally the failing Pharo-Integration tests but even after fixing a Sindarin loading error, the tests (testNoUnusedClassVariablesLeftand testObsoleteClasses) are green. This is how I load in a new Pharo 12 image:

Metacello new
	baseline: 'NewTools';
	repository: 'github://hernanmd/NewTools:add_dynamic_preview/src';
	load.

I also tried running through smalltalkCI but it was worst as Spec changes were missing:

bin/smalltalkci --headful -s Pharo64-12 ../git-repositories/NewTools/.smalltalk.ston

Do you know what else to try?

Ok, I managed to load the PR in the TravisCI.image and discovered lot of methods were missing, causing unreferenced variables and obsolete classes. I tried to rename the classes but still the Obsolete are introduced in the Pharo Integration CI job.

So this could mean code importing bug?

@hernanmd
Copy link
Member Author

hernanmd commented Feb 8, 2024

Finally. Errors were related to the {} + TraitedClass leftovers.
Now reported errors are not related:

LinkInstallerTest
 ✗ #testSlotOrVarLinksRemainAfterMethodModificationForObject (21ms)
 ✗ #testSlotOrVarLinksRemainAfterMethodModification (16ms)

RBRemoveProtocolTransformationTest
 ✗ #testTransform (1ms)

SocketStreamTest
 ✗ #testFlushLargeMessageOtherEndClosed (2ms)

RSDependencyTest
 ✗ #testDependencies (3ms)

StDebuggerTest
 ✗ #testUpdateExtensionSubscription (14ms)

OCTargetCompilerTest
 ✗ #testModifiedReturnFromClass (0ms)
  Executed 38664 Tests with 5 Failures and 2 Errors in 503.26s.
To reproduce the failed build locally, download smalltalkCI, and try to run something like:
	bin/smalltalkci -s Pharo64-12 --headfull /path/to/.smalltalk.ston

@hernanmd hernanmd requested a review from jecisc February 8, 2024 17:21
@Ducasse
Copy link
Contributor

Ducasse commented Feb 8, 2024

Tx

@Ducasse Ducasse merged commit 67c1901 into pharo-spec:Pharo12 Feb 8, 2024
1 of 2 checks passed
@MarcusDenker MarcusDenker changed the title Add dynamic preview File Dialog: Add dynamic preview Feb 9, 2024
@hernanmd hernanmd deleted the add_dynamic_preview branch July 6, 2024 09:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants