[GitHub] struts issue #158: WW-4835: Configurable handlers

2017-08-03 Thread cnenning
Github user cnenning commented on the issue: https://github.com/apache/struts/pull/158 sounds great 👍 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so

[GitHub] struts issue #158: WW-4835: Configurable handlers

2017-08-03 Thread cnenning
Github user cnenning commented on the issue: https://github.com/apache/struts/pull/158 and what is your idea? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes

[GitHub] struts issue #158: WW-4835: Configurable handlers

2017-08-03 Thread cnenning
Github user cnenning commented on the issue: https://github.com/apache/struts/pull/158 Do you want to further develop this PR or merge it first? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does

[GitHub] struts issue #158: WW-4835: Configurable handlers

2017-08-03 Thread cnenning
Github user cnenning commented on the issue: https://github.com/apache/struts/pull/158 Using `AbstractContentTypeHandler` to implement deprecated methods to log on WARN seems like a good idea. I don't see where new parameter `ActionInvocation` is actually used. Is that yet

[GitHub] struts issue #44: WW-4520 - Add prefix for CSS classes.

2017-08-02 Thread cnenning
Github user cnenning commented on the issue: https://github.com/apache/struts/pull/44 > I wonder how hard it will be to make this configurable Making this configurable would be cool. But when looking at changed files, and all those different file types, i fear it wo

[GitHub] struts issue #142: WW-4805 Blocks ognl access to class members of Spring pro...

2017-06-19 Thread cnenning
Github user cnenning commented on the issue: https://github.com/apache/struts/pull/142 IMO this can be merged --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes

[GitHub] struts issue #138: WW-3171 WW-3650 WW-4581: Locale aware converters

2017-05-18 Thread cnenning
Github user cnenning commented on the issue: https://github.com/apache/struts/pull/138 👍 for merging --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so

[GitHub] struts issue #138: WW-3171 WW-3650 WW-4581: Locale aware converters

2017-05-16 Thread cnenning
Github user cnenning commented on the issue: https://github.com/apache/struts/pull/138 > can you elaborate a bit more about parsing dates? I thought this is already supported. Now that you mention it, I see there is a locale aware `DateConverter`. Cannot remember why

[GitHub] struts issue #138: WW-3171 WW-3650 WW-4581: Locale aware converters

2017-05-16 Thread cnenning
Github user cnenning commented on the issue: https://github.com/apache/struts/pull/138 In our apps we have often demand for locale aware values. But more often we need dates, not doubles. Mostly we do this by calling java methods in JSPs. But having converts which do this out

[GitHub] struts issue #130: WW-3952: Credit card validator

2017-04-20 Thread cnenning
Github user cnenning commented on the issue: https://github.com/apache/struts/pull/130 If patterns change users have another reason to upgrade to latest struts 😆 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well

[GitHub] struts issue #130: WW-3952: Credit card validator

2017-04-20 Thread cnenning
Github user cnenning commented on the issue: https://github.com/apache/struts/pull/130 How often do credit card companies change their number-patterns in a way that the regex needs to be updated? 'guess it is rare enough. --- If your project is set up for it, you can reply

[GitHub] struts issue #131: WW-4210: Type conversion class

2017-04-20 Thread cnenning
Github user cnenning commented on the issue: https://github.com/apache/struts/pull/131 👍 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so

[GitHub] struts issue #130: WW-3952: Credit card validator

2017-04-20 Thread cnenning
Github user cnenning commented on the issue: https://github.com/apache/struts/pull/130 I hope the regex is stable 😉 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled

[GitHub] struts issue #128: WW-4578: Multidimensional validation

2017-04-10 Thread cnenning
Github user cnenning commented on the issue: https://github.com/apache/struts/pull/128 IMO these 4 could be considered, too: - `DoubleRangeFieldValidator` - `URLValidator` - `RequiredStringValidator` - `StringLengthFieldValidator` --- If your project is set up

[GitHub] struts issue #125: Immutable context

2017-03-24 Thread cnenning
Github user cnenning commented on the issue: https://github.com/apache/struts/pull/125 Sounds like a very good idea! A short check showed that my apps are not affected 😆 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub

[GitHub] struts issue #124: WW-4744: AnnotationUtils supports non-public methods

2017-03-21 Thread cnenning
Github user cnenning commented on the issue: https://github.com/apache/struts/pull/124 > Should I ask legal [1] about that? What do you think? That's the class that has been copied from spring. Yes, I think it's better to ask how to handle this. --- If your project is

[GitHub] struts issue #120: WW-4753: Injectable context

2017-03-10 Thread cnenning
Github user cnenning commented on the issue: https://github.com/apache/struts/pull/120 Besides the failing test this looks good to me --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have

[GitHub] struts issue #114: Virtual file representation

2016-11-24 Thread cnenning
Github user cnenning commented on the issue: https://github.com/apache/struts/pull/114 Looks good to me 👍 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes

[GitHub] struts issue #113: WW-4636 - File upload error message always in default lan...

2016-10-17 Thread cnenning
Github user cnenning commented on the issue: https://github.com/apache/struts/pull/113 :+1: That means #97 can be closed? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have

[GitHub] struts issue #108: ConversionErrorInterceptor to extend MethodFilterIntercep...

2016-09-30 Thread cnenning
Github user cnenning commented on the issue: https://github.com/apache/struts/pull/108 > How would this be fixed now? I fixed it already just by running `git cherry-pick`. (there is a jira comment about it) I feared that monstrous issue ([WW-4628](ht

[GitHub] struts issue #98: WW-4638 - TestNG 6.9.10 dependency error

2016-06-27 Thread cnenning
Github user cnenning commented on the issue: https://github.com/apache/struts/pull/98 > I think we should move portlet-app away from Struts into Struts Examples Oh, yes! We really should! --- If your project is set up for it, you can reply to this email and have your re

[GitHub] struts issue #99: add allowed methods to handleUnknownAction

2016-06-10 Thread cnenning
Github user cnenning commented on the issue: https://github.com/apache/struts/pull/99 Please create a jira ticket, too. We usually document all changes in jira to generate change logs. --- If your project is set up for it, you can reply to this email and have your reply appear

[GitHub] struts issue #99: add allowed methods to handleUnknownAction

2016-06-10 Thread cnenning
Github user cnenning commented on the issue: https://github.com/apache/struts/pull/99 > Ideally, line 224 would use the globalAllowedMethods set from the PackageConfig instead of a hard coded list but there is currently no associated getter method and I figured it would be bet

[GitHub] struts pull request: WW-4634 Centre alignment doesn't seem to work...

2016-05-25 Thread cnenning
Github user cnenning commented on the pull request: https://github.com/apache/struts/pull/95#issuecomment-221490265 :+1: --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled

[GitHub] struts pull request: WW-4594: Configure TilesDefs by annotating Ac...

2016-02-03 Thread cnenning
Github user cnenning commented on the pull request: https://github.com/apache/struts/pull/85#issuecomment-179082240 Alright, thanks for reviewing. I agree with your findings and pushed updates. I'm going to merge it later. --- If your project is set up for it, you can reply

[GitHub] struts pull request: WW-4594: Configure TilesDefs by annotating Ac...

2016-01-25 Thread cnenning
Github user cnenning commented on a diff in the pull request: https://github.com/apache/struts/pull/85#discussion_r50693282 --- Diff: plugins/tiles/src/test/java/org/apache/struts2/tiles/TestStrutsTilesAnnotationProcessor.java --- @@ -0,0 +1,148 @@ +package org.apache.struts2

[GitHub] struts pull request: WW-4594: Configure TilesDefs by annotating Ac...

2016-01-25 Thread cnenning
GitHub user cnenning opened a pull request: https://github.com/apache/struts/pull/85 WW-4594: Configure TilesDefs by annotating Actions Adds annotations for each element from `tiles.xml` to annotate actions. Those annotations are processed by a new class in tiles-plugin which

[GitHub] struts pull request: New result 'JSONActionRedirectResult' in json...

2016-01-19 Thread cnenning
Github user cnenning commented on the pull request: https://github.com/apache/struts/pull/72#issuecomment-172772942 > Merged #72. Yay, I got it landed :) This Email was scanned by Sophos Anti Virus --- If your project is set up for it, you can re

[GitHub] struts pull request: WW-4584: Upgrade tiles plugin

2016-01-18 Thread cnenning
Github user cnenning commented on the pull request: https://github.com/apache/struts/pull/73#issuecomment-172460318 > Simplifies logging Still not final :wink: ? Let's merge it! --- If your project is set up for it, you can reply to this email and have your re

[GitHub] struts pull request: WW-4584: Upgrade tiles plugin

2016-01-15 Thread cnenning
Github user cnenning commented on the pull request: https://github.com/apache/struts/pull/73#issuecomment-171968504 :+1: --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled

[GitHub] struts pull request: New result 'JSONActionRedirectResult' in json...

2016-01-14 Thread cnenning
Github user cnenning commented on a diff in the pull request: https://github.com/apache/struts/pull/72#discussion_r49698088 --- Diff: plugins/json/src/main/java/org/apache/struts2/json/JSONValidationInterceptor.java --- @@ -72,8 +74,8 @@ private static final Logger

[GitHub] struts pull request: WW-4584: Upgrade tiles plugin

2016-01-14 Thread cnenning
Github user cnenning commented on the pull request: https://github.com/apache/struts/pull/73#issuecomment-171662399 > I have resolved the problem with missing definitions but I'm a bit confused how Tiles resolves > resources - 66d29d4 - basically all the definitions are

[GitHub] struts pull request: New result 'JSONActionRedirectResult' in json...

2016-01-12 Thread cnenning
Github user cnenning commented on the pull request: https://github.com/apache/struts/pull/72#issuecomment-170908683 > Why static? There was no reason. In the app where that result was originaly implemented there was some other class calling that methods. I chan

[GitHub] struts pull request: WW-4584: Upgrade tiles plugin

2016-01-12 Thread cnenning
Github user cnenning commented on the pull request: https://github.com/apache/struts/pull/73#issuecomment-170932000 :+1: Looks great! There is one thing: when I try to run showcase app I get exceptions when trying to access tiles examples

[GitHub] struts pull request: New result 'JSONActionRedirectResult' in json...

2016-01-11 Thread cnenning
GitHub user cnenning opened a pull request: https://github.com/apache/struts/pull/72 New result 'JSONActionRedirectResult' in json-plugin Adds new result 'JSONActionRedirectResult' to json-plugin. Contains tests and example in showcase app. The new result type is intended

[GitHub] struts pull request: Struts 2.3.x: Tiles plugin upgrade

2016-01-07 Thread cnenning
Github user cnenning commented on the pull request: https://github.com/apache/struts/pull/60#issuecomment-169613075 I would say we merge this and may fix the "EL not working in freemarker-insert with freemarker-template" issue in master. --- If your project is set up for i

[GitHub] struts pull request: Struts 2.3.x: Tiles plugin upgrade

2016-01-05 Thread cnenning
Github user cnenning commented on the pull request: https://github.com/apache/struts/pull/60#issuecomment-168964296 > https://github.com/lukaszlenart/struts2-tiles-demo I created a PR for that demo app which demonstrates the issue. See https://github.com/lukaszlen

[GitHub] struts pull request: Struts 2.3.x: Tiles plugin upgrade

2015-12-21 Thread cnenning
Github user cnenning commented on the pull request: https://github.com/apache/struts/pull/60#issuecomment-166308691 The new version looks awesome! Just a small addon to constructor of `StrutsWildcardServletTilesApplicationContext`, that re-enables loading of `tiles*.xml

[GitHub] struts pull request: Struts 2.3.x: Tiles plugin upgrade

2015-11-30 Thread cnenning
Github user cnenning commented on the pull request: https://github.com/apache/struts/pull/60#issuecomment-160655820 For struts 2.3 I still got an old app with tiles2 which does some custom stuff and which has issues with these changes. * `tiles-servlet-wildcard` depends

[GitHub] struts pull request: Upgrade tiles plugin

2015-11-27 Thread cnenning
Github user cnenning commented on the pull request: https://github.com/apache/struts/pull/59#issuecomment-160149460 I hope that code can be shared between both tiles plugins. What do you think of creating a strust-tiles-commons.jar or somthing the like? --- If your project is set up

[GitHub] struts pull request: Upgrade tiles plugin

2015-11-27 Thread cnenning
Github user cnenning commented on the pull request: https://github.com/apache/struts/pull/59#issuecomment-160158437 yay, it runs ! :smiley: --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have