You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@struts.apache.org by GitBox <gi...@apache.org> on 2020/12/12 21:03:54 UTC

[GitHub] [struts-site] JCgH4164838Gh792C124B5 opened a new pull request #154: Possible site tag documentation updates for WW-5093.

JCgH4164838Gh792C124B5 opened a new pull request #154:
URL: https://github.com/apache/struts-site/pull/154


   Possible site tag documentation updates for WW-5093.
   - Minor updates to set, text and url tag text, to clarify scope and behaviour.
   - Minor clarifications on "accessing-application-session-request-objects", plus a few other minor edits.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org


[GitHub] [struts-site] lukaszlenart commented on a change in pull request #154: Possible site tag documentation updates for WW-5093.

Posted by GitBox <gi...@apache.org>.
lukaszlenart commented on a change in pull request #154:
URL: https://github.com/apache/struts-site/pull/154#discussion_r541911157



##########
File path: source/core-developers/accessing-application-session-request-objects.md
##########
@@ -50,16 +50,24 @@ is an alternative way to access the request and response objects, with the same
 ## Accessing from the view (JSP, FreeMarker, etc.)
 
 Request and session attributes are accessed via OGNL using the `#session` and `#request` stack values.
+Page attributes are accessed via OGNL using the `#attr` stack value, and Application attributes via
+the `#application` stack value.
 
 The `#attr` stack value will search the `javax.servlet.jsp.PageContext` for the specified key. If the `PageContext`
-doean't exist, it will search the request, session, and application scopes, in that order.
+doesn't exist, it will search the request, session, and application scopes, in that order.
 
-**Accessing the Session or Request from a JSP**
+**Accessing attributes in the Application, Session, Request, or Page scope from a JSP**
 
 ```jsp
+Retrieve the attribute (property), with key myId, from the specified scope:

Review comment:
       This is a JSP page, not XML. I would rather wrap this line (and the on below) with `<p></p>`




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org


[GitHub] [struts-site] lukaszlenart commented on pull request #154: Possible site tag documentation updates for WW-5093.

Posted by GitBox <gi...@apache.org>.
lukaszlenart commented on pull request #154:
URL: https://github.com/apache/struts-site/pull/154#issuecomment-748813722


   Looks good to me, LGTM 👍 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org


[GitHub] [struts-site] yasserzamani commented on pull request #154: Possible site tag documentation updates for WW-5093.

Posted by GitBox <gi...@apache.org>.
yasserzamani commented on pull request #154:
URL: https://github.com/apache/struts-site/pull/154#issuecomment-743991610


   Thanks a lot! BTW with thanks to @lukaszlenart , you can review a staged site at https://struts.staged.apache.org/ which reflects the PR changes I think (That's also visible on this PR's `Show all checks` link at bottom).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org


[GitHub] [struts-site] lukaszlenart commented on a change in pull request #154: Possible site tag documentation updates for WW-5093.

Posted by GitBox <gi...@apache.org>.
lukaszlenart commented on a change in pull request #154:
URL: https://github.com/apache/struts-site/pull/154#discussion_r541910942



##########
File path: source/tag-developers/set-tag.md
##########
@@ -25,9 +25,10 @@ The scopes available are as follows:
 - `session` - the value will be set in session scope according to servlet spec. using the name as key
 - `request` - the value will be set in request scope according to servlet spec. using the name as key
 - `page` - the value will be set in page scope according to servlet spec. using the name as key
-- `action` - the value will be set in the request scope and Struts' action context using the name as key
+- `action` - the value will be set in the page scope and Struts' action context using the name as key

Review comment:
       Not really, as you can chain actions so this will change which one is on the top of ValueStack.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org


[GitHub] [struts-site] JCgH4164838Gh792C124B5 commented on pull request #154: Possible site tag documentation updates for WW-5093.

Posted by GitBox <gi...@apache.org>.
JCgH4164838Gh792C124B5 commented on pull request #154:
URL: https://github.com/apache/struts-site/pull/154#issuecomment-743875878


   Hi.  I have not attempted any documentation site update PRs before, so hopefully the styling is OK.  If anything needs to be changed, please let me know.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org


[GitHub] [struts-site] JCgH4164838Gh792C124B5 commented on pull request #154: Possible site tag documentation updates for WW-5093.

Posted by GitBox <gi...@apache.org>.
JCgH4164838Gh792C124B5 commented on pull request #154:
URL: https://github.com/apache/struts-site/pull/154#issuecomment-748528953


   Hello @yasserzamani and @lukaszlenart .
   
   Thanks for the information on the staged site, as that really helps confirming the output.  😄 
   I applied the paragraph blocking that was requested, and made an extra mention of "attr" on the set-tag page along with the page-scope items.  I think the latter might help developers more easily remember the association for attr and page-scope that way ?
   
   Let me know if it looks OK now, or if more changes would improve it.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org


[GitHub] [struts-site] JCgH4164838Gh792C124B5 edited a comment on pull request #154: Possible site tag documentation updates for WW-5093.

Posted by GitBox <gi...@apache.org>.
JCgH4164838Gh792C124B5 edited a comment on pull request #154:
URL: https://github.com/apache/struts-site/pull/154#issuecomment-748528953


   Hello @yasserzamani and @lukaszlenart .
   
   Thanks for the information on the staged site, as that really helps confirming the output.  😄 
   I applied the paragraph blocking that was requested, and made an extra mention of "attr" on the set-tag page along with the page-scope items.  I think the latter might help developers more easily remember the association for attr and page-scope that way ?
   
   Let me know if it looks OK now, or if more changes would improve it.
   
   ... it looks like the CI build failed (some sort of GHFileNotFoundException), but does not seem to be related to the latest commit for this PR ...


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org


[GitHub] [struts-site] yasserzamani commented on a change in pull request #154: Possible site tag documentation updates for WW-5093.

Posted by GitBox <gi...@apache.org>.
yasserzamani commented on a change in pull request #154:
URL: https://github.com/apache/struts-site/pull/154#discussion_r541903584



##########
File path: source/core-developers/accessing-application-session-request-objects.md
##########
@@ -50,16 +50,24 @@ is an alternative way to access the request and response objects, with the same
 ## Accessing from the view (JSP, FreeMarker, etc.)
 
 Request and session attributes are accessed via OGNL using the `#session` and `#request` stack values.
+Page attributes are accessed via OGNL using the `#attr` stack value, and Application attributes via
+the `#application` stack value.
 
 The `#attr` stack value will search the `javax.servlet.jsp.PageContext` for the specified key. If the `PageContext`
-doean't exist, it will search the request, session, and application scopes, in that order.
+doesn't exist, it will search the request, session, and application scopes, in that order.
 
-**Accessing the Session or Request from a JSP**
+**Accessing attributes in the Application, Session, Request, or Page scope from a JSP**
 
 ```jsp

Review comment:
       xml is better I think to highlight.

##########
File path: source/core-developers/accessing-application-session-request-objects.md
##########
@@ -50,16 +50,24 @@ is an alternative way to access the request and response objects, with the same
 ## Accessing from the view (JSP, FreeMarker, etc.)
 
 Request and session attributes are accessed via OGNL using the `#session` and `#request` stack values.
+Page attributes are accessed via OGNL using the `#attr` stack value, and Application attributes via
+the `#application` stack value.
 
 The `#attr` stack value will search the `javax.servlet.jsp.PageContext` for the specified key. If the `PageContext`
-doean't exist, it will search the request, session, and application scopes, in that order.
+doesn't exist, it will search the request, session, and application scopes, in that order.
 
-**Accessing the Session or Request from a JSP**
+**Accessing attributes in the Application, Session, Request, or Page scope from a JSP**
 
 ```jsp
+Retrieve the attribute (property), with key myId, from the specified scope:

Review comment:
       Nice to use xml comment here i.e. `<!-- .... -->`

##########
File path: source/tag-developers/set-tag.md
##########
@@ -25,9 +25,10 @@ The scopes available are as follows:
 - `session` - the value will be set in session scope according to servlet spec. using the name as key
 - `request` - the value will be set in request scope according to servlet spec. using the name as key
 - `page` - the value will be set in page scope according to servlet spec. using the name as key
-- `action` - the value will be set in the request scope and Struts' action context using the name as key
+- `action` - the value will be set in the page scope and Struts' action context using the name as key

Review comment:
       I think action would be request (thread) scope theoretically?

##########
File path: source/core-developers/accessing-application-session-request-objects.md
##########
@@ -50,16 +50,24 @@ is an alternative way to access the request and response objects, with the same
 ## Accessing from the view (JSP, FreeMarker, etc.)
 
 Request and session attributes are accessed via OGNL using the `#session` and `#request` stack values.
+Page attributes are accessed via OGNL using the `#attr` stack value, and Application attributes via
+the `#application` stack value.
 
 The `#attr` stack value will search the `javax.servlet.jsp.PageContext` for the specified key. If the `PageContext`
-doean't exist, it will search the request, session, and application scopes, in that order.
+doesn't exist, it will search the request, session, and application scopes, in that order.
 
-**Accessing the Session or Request from a JSP**
+**Accessing attributes in the Application, Session, Request, or Page scope from a JSP**
 
 ```jsp
+Retrieve the attribute (property), with key myId, from the specified scope:
+
+<s:property value="#application.myId" />
+
 <s:property value="#session.myId" />
 
 <s:property value="#request.myId" />
 
 <s:property value="#attr.myId" />
+
+Reminder: attr is for Page scope attributes first, but will search the remaining scopes, in order, seeking a match.

Review comment:
       (same here)




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org


[GitHub] [struts-site] lukaszlenart merged pull request #154: Possible site tag documentation updates for WW-5093.

Posted by GitBox <gi...@apache.org>.
lukaszlenart merged pull request #154:
URL: https://github.com/apache/struts-site/pull/154


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org