Skip to content

Conversation

Zemogiter
Copy link
Contributor

@Zemogiter Zemogiter commented Jul 9, 2019

Description

In it's current shape, the ZipScript dosen't do much because there is no information regarding the .exe file inside archive.

What works

to be filled

What was not tested

Nothing was tested yet, waiting for @ImperatorS79 to test this in his Adobe Photoshop script #1122

Test

  • Operating system (and linux kernel version):
  • Hardware (GPU/CPU):

Ready for review

  • Script tested as a regular phoenicis user and working (if you have a problem -> create as draft and ask for help).
  • json-align and eslint run according to the documentation.
  • Codacy and travis checked.

@plata
Copy link
Collaborator

plata commented Jul 26, 2019

Can you maybe test with some other free application so that we are not blocked by Photoshop?

@Zemogiter
Copy link
Contributor Author

I could but I don't know any application other than Photoshop that would make use of ZipScript.

Copy link
Collaborator

@madoar madoar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Zemogiter the script requires some formatting

Add a file existence check.
@Zemogiter
Copy link
Contributor Author

@madoar is this the check sufficient?

@madoar
Copy link
Collaborator

madoar commented Sep 7, 2019

I think you should invert the check, i.e. add behavior in case the file does not exist. I think ideally we should show an error message informing the user of the problem.

@Zemogiter
Copy link
Contributor Author

@madoar done

@ImperatorS79 ImperatorS79 marked this pull request as ready for review September 8, 2019 15:01
This was referenced Sep 10, 2019
@ImperatorS79
Copy link
Contributor

I get using this that the file must be in Phoenicis root directory (the _setupPath is "Adobe Photoshop/Set-up.exe" so that wine.runInsidePrefix() launch drive_c/Adobe Photoshop/Set-Up.exe)

@plata
Copy link
Collaborator

plata commented Sep 10, 2019

Can you show the exact error message? I'm assuming that a prefix is lost at some point.

@madoar madoar requested review from plata and ImperatorS79 October 5, 2019 20:09
Copy link
Collaborator

@plata plata left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you check that both cases still work (with and without setup path)?

@madoar madoar requested review from ImperatorS79 and plata November 25, 2019 21:23
@ImperatorS79
Copy link
Contributor

I do not have time to review currently.

@plata plata merged commit b5f395c into PhoenicisOrg:master Nov 26, 2019
@Zemogiter Zemogiter deleted the ZipScript-fix branch November 26, 2019 22:04
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