On Wed, 24 Aug 2022 18:34:41 GMT, Tejesh R <[email protected]> wrote:

> > > > Although I've reviewed the CSR I'd prefer it not be finalized yet. I 
> > > > think it needs more changes. The spec is SILENT about what then happens 
> > > > if length is negative and SILENT about an out of range offset too. I 
> > > > think we should address these issues as well.
> > > > Also everywhere - in the CSR and the bug description and summary the 
> > > > text said the method is called setCharacterAttribute whereas it is 
> > > > setCharacterAttributes.
> > > 
> > > 
> > > For negative length, now it just returns the control without processing 
> > > it whereas before it used to raise an exception in arrayCopy method. 
> > > Upper bound for length should be the text length(which is to be 
> > > modified/added) I feel, still even if it crosses overall text size that 
> > > case is handled by limiting to text length. These are not mentioned in 
> > > spec, so should we modify the spec by adding the range bounds for 
> > > length......?
> > 
> > 
> > Yes I am saying we should mention all of this
> 
> Will this addition to spec be fine -
> 
> ```
>       * A write lock is held by this operation while changes
>       * are being made, and a DocumentEvent is sent to the listeners
>       * after the change has been successfully completed.
> +     *
> +     * <p>
> +     * The expected {@Code length} range is the length of the text
> +     * in which the attributes are set. If the length is &lt;= 0, then no
> +     * attributes are set, the control returns. If the length is &gt; the
> +     * length of text in which the attributes are to be set then the
> +     * extra length is trimmed.
> +     * </p>
> +     *
>       * <p>
>       * This method is thread safe, although most Swing methods
>       * are not. Please see
> ```

should be {@code .. } 
And the control doesn't return, the method does. I think you were trying to say 
control returns to the caller but that doesn't work here and the interaction 
with offset is cryptic .. assuming that's the reference to "trimmed".
Something that makes clear that the replace arg isn't used in such a case might 
help too.

I expect something like

     * {@code offset} and {@code length} define the range of the text over 
which the attributes are set.
     * If the length is &lt;= 0, then no action is taken  and the method just 
returns.
     * If the offset is < 0 or >= the length of the text then no action is 
taken, and the method just returns
     *  Otherwise if {@code offset + length} will exceed the length of the  
text then the affected range is truncated to that length
     * 
But YOU  need to verify this is all actually true ..

-------------

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

Reply via email to