Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Added support for single element pages#4

Merged
langecode merged 20 commits intoopenapi-tools:masteropenapi-tools/api-capabilities:masterfrom
Trackunit:masterTrackunit/api-capabilities:masterCopy head branch name to clipboard
Nov 8, 2019
Merged

Added support for single element pages#4
langecode merged 20 commits intoopenapi-tools:masteropenapi-tools/api-capabilities:masterfrom
Trackunit:masterTrackunit/api-capabilities:masterCopy head branch name to clipboard

Conversation

@JesperRossen
Copy link

Changes:

  • Added support for single element spans (eg. 1|1 or 500|500)
  • Changed regex to only accept a full from/to input string, compared to before where it also accepted a single number, which wasn't actually supported in the code
  • Updated tests to support new input

Since both from and to is inclusive, as stated in the readme, it should be a valid input to be able to say 1|1, for example if you only want the first element of a list, or if you're generating links for the previous/next page and you end up having the last page only one element long.

@codecov
Copy link

codecov bot commented Oct 14, 2019

Codecov Report

Merging #4 into master will increase coverage by 0.77%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master       #4      +/-   ##
============================================
+ Coverage     89.74%   90.51%   +0.77%     
- Complexity      105      106       +1     
============================================
  Files            10       10              
  Lines           234      232       -2     
  Branches         35       35              
============================================
  Hits            210      210              
+ Misses            8        6       -2     
  Partials         16       16
Impacted Files Coverage Δ Complexity Δ
...java/io/openapitools/api/capabilities/Element.java 100% <100%> (+9.52%) 11 <10> (+1) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fed7a77...8bff051. Read the comment docs.

@langecode
Copy link
Member

Sounds reasonable. I will take a look and possibly merge this week.

if (null == element) {
return Optional.empty();
}
if (!element.matches("^([0-9]+)?(\\|[0-9]+)?")) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wonder if the syntax of selecting just one element would most naturally be 1|1 or just 1 - it seems for some reason originally both the first part and the last part was optional. If the last part was kept optional you could still allow for a selection in its most simple form 1

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I adjusted the code to allow both single element, and same start and end element.

@langecode
Copy link
Member

Looks good - but I do believe that maybe it should be allowed to leave out the | also - would at least look more nice.

@gringostar
Copy link
Collaborator

Looks good - but I do believe that maybe it should be allowed to leave out the | also - would at least look more nice.

Done. Updated the readme to reflect this.

@langecode
Copy link
Member

Thanks 👍

@langecode langecode self-requested a review November 8, 2019 11:05
@langecode langecode merged commit 7522964 into openapi-tools:master Nov 8, 2019
@gringostar
Copy link
Collaborator

Hi @langecode. Are you planning a new release with the merged changes?

@langecode
Copy link
Member

Indeed this is released as 1.0.4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.