Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix bookmark integer overflow and Notes UI bug #165

Merged
merged 10 commits into from
Nov 2, 2020

Conversation

TanLeYang
Copy link
Collaborator

@TanLeYang TanLeYang commented Nov 2, 2020

Closes #163
Added length limit to the regex of the bookmark field to prevent integer overflow.
Wanted to use BigInteger but it would require changing the whole project to use BigInteger so just went with the regex solution.

Closes #162
Allow text to wrap around instead of being truncated into ellipses when the text is too long to be displayed in a single line.
Here's how it looks now with the maximum number of allowable characters for note and note title:
ss4

DG changes
Modified the DG for the undo/redo feature to include the new changes I added last week

@TanLeYang TanLeYang added this to the v1.4 milestone Nov 2, 2020
@TanLeYang TanLeYang changed the title V1.4bug fixes Fix bookmark integer overflow and Notes UI bug Nov 2, 2020
@codecov-io
Copy link

codecov-io commented Nov 2, 2020

Codecov Report

Merging #165 into master will increase coverage by 0.37%.
The diff coverage is 40.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #165      +/-   ##
============================================
+ Coverage     71.32%   71.69%   +0.37%     
  Complexity      745      745              
============================================
  Files           127      127              
  Lines          2420     2406      -14     
  Branches        271      269       -2     
============================================
- Hits           1726     1725       -1     
+ Misses          596      582      -14     
- Partials         98       99       +1     
Impacted Files Coverage Δ Complexity Δ
.../main/java/seedu/bookmark/model/book/Bookmark.java 88.88% <0.00%> (+3.88%) 10.00 <0.00> (ø)
...ain/java/seedu/bookmark/model/book/TotalPages.java 83.33% <ø> (ø) 9.00 <0.00> (ø)
src/main/java/seedu/bookmark/ui/BookCard.java 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
src/main/java/seedu/bookmark/ui/BookListPanel.java 0.00% <ø> (ø) 0.00 <0.00> (ø)
.../main/java/seedu/bookmark/ui/DetailedBookCard.java 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
.../java/seedu/bookmark/ui/DetailedBookListPanel.java 0.00% <ø> (ø) 0.00 <0.00> (ø)
src/main/java/seedu/bookmark/ui/NoteCard.java 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
src/main/java/seedu/bookmark/ui/SidebarPanel.java 0.00% <ø> (ø) 0.00 <0.00> (ø)
...ain/java/seedu/bookmark/logic/ViewTypeManager.java 78.57% <66.66%> (ø) 5.00 <2.00> (?)
...n/java/seedu/bookmark/commons/util/StringUtil.java 95.45% <100.00%> (ø) 8.00 <0.00> (ø)
... and 3 more

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 d8ab0e8...ea28373. Read the comment docs.

Copy link
Collaborator

@pangpuncake pangpuncake left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Collaborator

@angrybunny123 angrybunny123 left a comment

Choose a reason for hiding this comment

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

LGTM :)

Copy link
Collaborator

@pennhanlee pennhanlee left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@angrybunny123 angrybunny123 left a comment

Choose a reason for hiding this comment

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

LGTM other than the Big Integer part that I'm not too sure about

@@ -74,7 +75,7 @@ public static boolean isNonZeroUnsignedInteger(String s) {
*/
public static String trimLeadingZeroes(String s) {
requireNonNull(s);
int value = Integer.parseInt(s);
BigInteger value = new BigInteger(s);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure if this needs to be here as you mentioned in the PR that you were using the regex solution instead

@TanLeYang TanLeYang merged commit 9095412 into AY2021S1-CS2103T-F13-2:master Nov 2, 2020
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.

Integer overflow issues with bookmark [PE-D] Cannot display full description for note
5 participants