Github user martin-g commented on a diff in the pull request:

    https://github.com/apache/wicket/pull/219#discussion_r111096456
  
    --- Diff: 
wicket-core/src/main/java/org/apache/wicket/resource/FileSystemResource.java ---
    @@ -68,17 +68,19 @@ public FileSystemResource()
        @Override
        protected ResourceResponse newResourceResponse(Attributes attributes)
        {
    -           return createResourceResponse(path);
    +           return createResourceResponse(path, null);
        }
     
        /**
         * Creates a resource response based on the given attributes
         * 
         * @param path
         *            the path to create the resource response with
    +    * @param fileName
    +    *            fileName to set, path.getFileName() will be used in case 
null passed
         * @return the actual resource response x
         */
    -   protected ResourceResponse createResourceResponse(Path path)
    +   protected ResourceResponse createResourceResponse(Path path, String 
fileName)
    --- End diff --
    
    Do we really need this extra parameter here for the fileName ?
    IMO by default it should do: `if (fileName != null) 
resourceResponse.setFileName(path.getFileName().toString());`
    If the application wants to override it then do something like:
    ```java
    ResourceResponse rr = super.createResourceResponse(path);
    rr.setFileName("custom.name");
    return rr;
    ```
    
    Also I think passing `Attributes` as a parameter would be useful!


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Reply via email to