[jira] [Commented] (GROOVY-8251) Implement withCloseable on AutoCloseable

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/GROOVY-8251?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16090961#comment-16090961
 ] 

ASF GitHub Bot commented on GROOVY-8251:


Github user asfgit closed the pull request at:

https://github.com/apache/groovy/pull/572


> Implement withCloseable on AutoCloseable
> 
>
> Key: GROOVY-8251
> URL: https://issues.apache.org/jira/browse/GROOVY-8251
> Project: Groovy
>  Issue Type: Bug
>  Components: groovy-jdk
>Affects Versions: 2.5.0-beta-1
>Reporter: Henri Tremblay
>
> The Groovy implementation of try-with-resource is through 
> {{withCloseable()}}. The problem is that {{withCloseable}} only exists for 
> classes implementing the {{Closeable}} interface.
> But a try-with-resource works with the {{AutoCloseable}} interface. So 
> {{withCloseable}} should probably be moved on step below to {{AutoCloseable}} 
> (that {{Closeable}} extends).



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (GROOVY-8251) Implement withCloseable on AutoCloseable

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/GROOVY-8251?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16079406#comment-16079406
 ] 

ASF GitHub Bot commented on GROOVY-8251:


GitHub user jwagenleitner opened a pull request:

https://github.com/apache/groovy/pull/572

GROOVY-8251: Implement withCloseable on AutoCloseable

`NioGroovyMethods.withAutoCloseable` was added for 2.5.0 as part of 
[GROOVY-7572](https://issues.apache.org/jira/browse/GROOVY-7572).  It appears 
it was originally placed here because at the time Java 6 was still used, but 
now 2.5.0 is 7+.

First commit proposes moving the method (as-is) into the core 
`IOGroovyMethods` class.  This is where the 
`withCloseable(java.io.Closeable...)` method is, so also part of this change is 
moving the tests for it from NIO to core.  Since 2.5.0 is not been officially 
released I don't think there is a need to deprecate or worry about binary 
compatibility for the `withAutoCloseable` method.

Second commit proposes to rename the new but unreleased method from 
`withAutoCloseable` to `withCloseable` to match the existing method on 
`java.io.Closeable`.  See discussion on 
[GROOVY-8251](https://issues.apache.org/jira/browse/GROOVY-8251).

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/jwagenleitner/groovy 8251-AutoCloseable

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/groovy/pull/572.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #572


commit 912200153193849fc35a09a053898d46c7f1dacd
Author: John Wagenleitner 
Date:   2017-07-09T00:22:36Z

GROOVY-8251: Implement withCloseable on AutoCloseable

Relocate withAutoCloseable from NIO subproject to core. Since
AutoCloseable is not an NIO related class and release 2.5.0
will target Java 7, the method should be available as part
of core.

Relocated withCloseable tests to core since that method
is deprecated in the NIO module.

commit 261bc174980da8d6b6f4cdb54b8f0ab98c64c4fd
Author: John Wagenleitner 
Date:   2017-07-09T01:13:48Z

GROOVY-8251: rename withAutoCloseable to withCloseable




> Implement withCloseable on AutoCloseable
> 
>
> Key: GROOVY-8251
> URL: https://issues.apache.org/jira/browse/GROOVY-8251
> Project: Groovy
>  Issue Type: Bug
>  Components: groovy-jdk
>Affects Versions: 2.5.0-beta-1
>Reporter: Henri Tremblay
>
> The Groovy implementation of try-with-resource is through 
> {{withCloseable()}}. The problem is that {{withCloseable}} only exists for 
> classes implementing the {{Closeable}} interface.
> But a try-with-resource works with the {{AutoCloseable}} interface. So 
> {{withCloseable}} should probably be moved on step below to {{AutoCloseable}} 
> (that {{Closeable}} extends).



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (GROOVY-8251) Implement withCloseable on AutoCloseable

[Henri Tremblay (JIRA)]

[ 
https://issues.apache.org/jira/browse/GROOVY-8251?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16077025#comment-16077025
 ] 

Henri Tremblay commented on GROOVY-8251:


Yes. Because let's be fair, the only reason we have {{AutoCloseable}} today is 
because {{Closeable}} was typed {{IOException}}. Starting from scratch, we 
would only have {{Closeable}} throwing an exception.

> Implement withCloseable on AutoCloseable
> 
>
> Key: GROOVY-8251
> URL: https://issues.apache.org/jira/browse/GROOVY-8251
> Project: Groovy
>  Issue Type: Bug
>  Components: groovy-jdk
>Affects Versions: 2.5.0-beta-1
>Reporter: Henri Tremblay
>
> The Groovy implementation of try-with-resource is through 
> {{withCloseable()}}. The problem is that {{withCloseable}} only exists for 
> classes implementing the {{Closeable}} interface.
> But a try-with-resource works with the {{AutoCloseable}} interface. So 
> {{withCloseable}} should probably be moved on step below to {{AutoCloseable}} 
> (that {{Closeable}} extends).



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (GROOVY-8251) Implement withCloseable on AutoCloseable

[John Wagenleitner (JIRA)]

[ 
https://issues.apache.org/jira/browse/GROOVY-8251?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16077004#comment-16077004
 ] 

John Wagenleitner commented on GROOVY-8251:
---

My initial thought was sort of the same, just add a 
{{withCloseable(AutoCloseable..) throws Exception}} method.  I don't think 
there is any limitation, other than the old {{withCloseable(Closeable..) throws 
IOException}} method would probably need to be kept for binary compatibility.  
The {{throws}} clause of the old method would probably need to change but don't 
think that is a problem.

There may be some confusion with the name {{withCloseable}} taking either a 
{{Closeable}} or {{AutoCloseable}}, given that {{AutoCloseable}} is the more 
general type.  But I like the concise name {{withCloseable}} more.

> Implement withCloseable on AutoCloseable
> 
>
> Key: GROOVY-8251
> URL: https://issues.apache.org/jira/browse/GROOVY-8251
> Project: Groovy
>  Issue Type: Bug
>  Components: groovy-jdk
>Affects Versions: 2.5.0-beta-1
>Reporter: Henri Tremblay
>
> The Groovy implementation of try-with-resource is through 
> {{withCloseable()}}. The problem is that {{withCloseable}} only exists for 
> classes implementing the {{Closeable}} interface.
> But a try-with-resource works with the {{AutoCloseable}} interface. So 
> {{withCloseable}} should probably be moved on step below to {{AutoCloseable}} 
> (that {{Closeable}} extends).



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (GROOVY-8251) Implement withCloseable on AutoCloseable

[John Wagenleitner (JIRA)]

[ 
https://issues.apache.org/jira/browse/GROOVY-8251?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16076827#comment-16076827
 ] 

John Wagenleitner commented on GROOVY-8251:
---

Looks like GROOVY-7572 added a {{withAutoCloseable}} method in the NIO 
submodule and it's in 2.5.  I don't know if there was a reason for the 
different name, but I think it would make more sense to allow {{withCloseable}} 
to take either a {{Closeable}} or {{AutoCloseable}}.  I think to keep binary 
compatibility a new {{withCloseable}} method would have to be added and not 
changing the signature of the existing method.  Since 2.5 is not out yet (still 
in beta), depending on how others feel, I wonder if there's still time to make 
a change.

> Implement withCloseable on AutoCloseable
> 
>
> Key: GROOVY-8251
> URL: https://issues.apache.org/jira/browse/GROOVY-8251
> Project: Groovy
>  Issue Type: Bug
>  Components: groovy-jdk
>Affects Versions: 2.5.0-beta-1
>Reporter: Henri Tremblay
>
> The Groovy implementation of try-with-resource is through 
> {{withCloseable()}}. The problem is that {{withCloseable}} only exists for 
> classes implementing the {{Closeable}} interface.
> But a try-with-resource works with the {{AutoCloseable}} interface. So 
> {{withCloseable}} should probably be moved on step below to {{AutoCloseable}} 
> (that {{Closeable}} extends).



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)