Re: RFR: 8288838: jpackage: file association additional arguments [v3]

2022-06-29 Thread Alex Kasko
> jpackage implementation of file association on Windows currently passes a 
> selected filename as an only argument to associated executable.
> 
> It is proposed to introduce additional option in file association property 
> file to allow optionally support additional arguments using `%*` batch 
> wildcard.
> 
> Note, current implementation, while fully functional, is only a **DRAFT** 
> one, it is not ready for integration in this form. I would appreciate any 
> guidance on the following points:
> 
>  - option naming inside a properties file, currently `pass-all-args` is used
>  - option naming in a bundler parameter implementation, it is not clear if it 
> should introduce a new group of "file association windows specific options" 
> next to the existing "file association mac specific options" group
>  - test organization to cover the new option: currently it is included inside 
> `FileAssociationTest` and piggybacks on the existing (and unrelated) 
> `includeDescription` parameter; it is not clear whether it should be done in 
> a separate test and whether to include runs for every parameter combination
>  - test run implementation: currently arguments are checked when a file with 
> associated extension is invoked from command line; it is not clear whether it 
> would be more appropriate instead to create a desktop shortcut with the same 
> command as a target and to invoke it with `java.awt.Desktop`
> 
> Also please note, that full install/uninstall run is currently enabled in 
> `FileAssociationTest`, it is intended to be used only in a draft code during 
> the development and to be removed (to use the same "install or unpack" logic 
> as other tests) in a final version.
> 
> Testing:
>  
> - [x] test to cover new logic is included
> - [x] ran jtreg:jdk/tools/jpackage with no new failures

Alex Kasko has updated the pull request incrementally with one additional 
commit since the last revision:

  drop pass-all-args property

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/9224/files
  - new: https://git.openjdk.org/jdk/pull/9224/files/3b4f77e8..7e14f614

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=9224&range=02
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=9224&range=01-02

  Stats: 60 lines in 7 files changed: 0 ins; 37 del; 23 mod
  Patch: https://git.openjdk.org/jdk/pull/9224.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/9224/head:pull/9224

PR: https://git.openjdk.org/jdk/pull/9224


Re: RFR: 8288838: jpackage: file association additional arguments [v3]

2022-07-07 Thread Alexey Semenyuk
On Wed, 29 Jun 2022 12:29:33 GMT, Alex Kasko  wrote:

>> jpackage implementation of file association on Windows currently passes a 
>> selected filename as an only argument to associated executable.
>> 
>> It is proposed to introduce additional option in file association property 
>> file to allow optionally support additional arguments using `%*` batch 
>> wildcard.
>> 
>> Note, current implementation, while fully functional, is only a **DRAFT** 
>> one, it is not ready for integration in this form. I would appreciate any 
>> guidance on the following points:
>> 
>>  - option naming inside a properties file, currently `pass-all-args` is used
>>  - option naming in a bundler parameter implementation, it is not clear if 
>> it should introduce a new group of "file association windows specific 
>> options" next to the existing "file association mac specific options" group
>>  - test organization to cover the new option: currently it is included 
>> inside `FileAssociationTest` and piggybacks on the existing (and unrelated) 
>> `includeDescription` parameter; it is not clear whether it should be done in 
>> a separate test and whether to include runs for every parameter combination
>>  - test run implementation: currently arguments are checked when a file with 
>> associated extension is invoked from command line; it is not clear whether 
>> it would be more appropriate instead to create a desktop shortcut with the 
>> same command as a target and to invoke it with `java.awt.Desktop`
>> 
>> Also please note, that full install/uninstall run is currently enabled in 
>> `FileAssociationTest`, it is intended to be used only in a draft code during 
>> the development and to be removed (to use the same "install or unpack" logic 
>> as other tests) in a final version.
>> 
>> Testing:
>>  
>> - [x] test to cover new logic is included
>> - [x] ran jtreg:jdk/tools/jpackage with no new failures
>
> Alex Kasko has refreshed the contents of this pull request, and previous 
> commits have been removed. Incremental views are not available.

test/jdk/tools/jpackage/share/FileAssociationsTest.java line 108:

> 106: .applyTo(packageTest);
> 107: 
> 108: packageTest.run(RunnablePackageTest.Action.CREATE, 
> RunnablePackageTest.Action.INSTALL,

FYI: the default test steps can be overridden with the value of 
`jpackage.test.action` system property.
Its value would be `create,install,verify_install,uninstall` for this case.

UPD: Nevermind this comment. It applied to the old version of the PR.

-

PR: https://git.openjdk.org/jdk/pull/9224


Re: RFR: 8288838: jpackage: file association additional arguments [v3]

2022-07-07 Thread Alex Kasko
On Thu, 30 Jun 2022 18:50:23 GMT, Alexey Semenyuk  wrote:

>> Alex Kasko has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   drop pass-all-args property
>
> test/jdk/tools/jpackage/share/FileAssociationsTest.java line 108:
> 
>> 106: .applyTo(packageTest);
>> 107: 
>> 108: packageTest.run(RunnablePackageTest.Action.CREATE, 
>> RunnablePackageTest.Action.INSTALL,
> 
> FYI: the default test steps can be overridden with the value of 
> `jpackage.test.action` system property.
> Its value would be `create,install,verify_install,uninstall` for this case.
> 
> UPD: Nevermind this comment. It applied to the old version of the PR.

Thanks for the info! I've seen `jpackage.test.action` property in 
`run_tests.sh`, but never used it.

-

PR: https://git.openjdk.org/jdk/pull/9224