RFR: 8345348: CSS media feature queries [v13]

Andy Goryachev angorya at openjdk.org
Fri May 2 21:45:52 UTC 2025


On Mon, 14 Apr 2025 14:35:43 GMT, Michael Strauß <mstrauss at openjdk.org> wrote:

>> modules/javafx.graphics/src/main/java/com/sun/javafx/css/parser/TokenStream.java line 31:
>> 
>>> 29: import java.util.function.Predicate;
>>> 30: 
>>> 31: public final class TokenStream {
>> 
>> I think the naming of the methods in this class leaves something to be desired.
>> 
>> - `Token consume()` **OK**
>> - `Token consume(Predicate)` -> `consumeIf` or `consumeIfMatches`
>>     Makes it clearer, as `consume` seems to imply something is always consumed (ie. it still skips one token if the predicate didn't match returning `null`).
>> - `Token peek()` **OK**
>> - `reconsume` -> `unconsume`, `undoConsume`, `undo`, `previous`, `resetToPrevious`, `decrementIndex`
>>     Nothing is consumed which reconsume seems to imply, instead it moves the index back one place so the next call to `consume` may indeed reconsume a token; reconsume as-is would do nothing (and if it returned anything it would be same the as `current`.
>> - `boolean peekMany(Predicate...)` -> `matches`
>>     It doesn't work like `peek`.  `peekMany` would imply it returns a `List<Token>`; it also doesn't convey that it returns a `boolean`.
>> - `reset(int)` -> `setIndex`
>>     This seems to be similar to what say `InputStream` provides, but `InputStream` hides the `index` parameter so you can't set it to some arbitrary value (like skipping ahead).  If you want to mimic this pattern, I'd suggest removing the parameter and providing a `mark` method (or make it non-public).
>
> I renamed `consume(Predicate)` to `consumeIf(Predicate)`, and `peekMany(Predicate...)` to `matches(Predicate...)`.
> 
> `index()` and `reset()` don't need to be public, so I've removed that (also I don't want to mimic the problematic mark/reset pattern of `InputStream`).
> 
> `reconsume` is [CSS lingo](https://www.w3.org/TR/css-syntax-3/#tokenizer-definitions), I'm keeping that as-is.

`reconsume` -> `resetToPrevious` + mention "reconsume" in javadoc?

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1655#discussion_r2072177991


More information about the openjfx-dev mailing list