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

Allow to remove all checked items (fix #1521) #1773

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,10 @@ public boolean onOptionsItemSelected(MenuItem item) {
} else if (itemId == R.id.menu_title) {
showEditTitleDialog();
return true;
} else if (itemId == R.id.menu_move) {
} else if (itemId == R.id.menu_remove_checked_items) {
removeCheckedItems();
return true;
} else if (itemId == R.id.menu_move) {
executor.submit(() -> AccountPickerDialogFragment
.newInstance(new ArrayList<>(repo.getAccounts()), note.getAccountId())
.show(requireActivity().getSupportFragmentManager(), BaseNoteFragment.class.getSimpleName()));
Expand Down Expand Up @@ -345,7 +348,7 @@ private void showCategorySelector() {
}

/**
* Opens a dialog in order to chose a category
* Opens a dialog in order to edit the title
*/
public void showEditTitleDialog() {
saveNote(null);
Expand All @@ -360,6 +363,28 @@ public void showEditTitleDialog() {
editTitleFragment.show(manager, fragmentId);
}

/**
* Removes all checked items from a note
*
* @return
*/
public String removeCheckedItems() {
Log.d(TAG, "removeCheckedItems()");
if (note != null) {
final var oldContent = note.getContent();
final var newContent = NoteUtil.getContentWithoutCheckedItems(oldContent);
if (!oldContent.equals(newContent)) {
note.setContent(newContent);
}

return newContent;
} else {
Log.e(TAG, "note is null");
}

return "";
}

@Override
public void onCategoryChosen(String category) {
repo.setCategory(localAccount, note.getId(), category);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,16 @@ protected void colorWithText(@NonNull String newText, @Nullable Integer current,
}
}

public String removeCheckedItems() {
String newContent = super.removeCheckedItems();

if (note != null) {
binding.editContent.setMarkdownString(newContent);
}

return newContent;
}

@Override
public void applyBrand(int color) {
super.applyBrand(color);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,16 @@ protected void colorWithText(@NonNull String newText, @Nullable Integer current,
}
}

public String removeCheckedItems() {
String newContent = super.removeCheckedItems();

if (note != null) {
binding.singleNoteContent.setMarkdownString(newContent);
}

return newContent;
}

@Override
protected String getContent() {
return changedText;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ public void onPrepareOptionsMenu(@NonNull Menu menu) {
menu.findItem(R.id.menu_share).setVisible(false);
menu.findItem(R.id.menu_move).setVisible(false);
menu.findItem(R.id.menu_category).setVisible(false);
menu.findItem(R.id.menu_remove_checked_items).setVisible(false);
menu.findItem(R.id.menu_title).setVisible(false);
if (menu.findItem(MENU_ID_PIN) != null)
menu.findItem(MENU_ID_PIN).setVisible(false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@
import static it.niedermann.android.markdown.MarkdownUtil.removeMarkdown;
import static it.niedermann.android.markdown.MarkdownUtil.replaceCheckboxesWithEmojis;

import java.util.regex.Matcher;
import java.util.regex.Pattern;

/**
* Provides basic functionality for Note operations.
* Created by stefan on 06.10.15.
Expand Down Expand Up @@ -123,6 +126,29 @@ public static String getLineWithoutMarkdown(@NonNull String content, int lineNum
return line;
}

/**
* Strips all lines beginning with a filled checkbox from the Markdown.
*
* @param content String
* @return newContent String
*/
@NonNull
public static String getContentWithoutCheckedItems(@NonNull String content) {
final Pattern pattern = Pattern.compile(
"^ *[-+*] \\[[xX]\\].*$",
Pattern.MULTILINE
);
Copy link
Member

Choose a reason for hiding this comment

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

This omits some variants that could lead to undefined behavior, for example checkboxes within code blocks. You can get some inspiration how to handle codeblocks in the nextcloud-commons library, but beware: It ain't that easy. Codeblocks can have three or more backticks, for example

Copy link
Author

Choose a reason for hiding this comment

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

Thanks, I will!

final Matcher matcher = pattern.matcher(content);

// We can't just replace the matcher with an empty string as this would
// let a blank line. To be able to remove the blank line as well, we set
// a random-ish token that we will then match with the EOL as well and
// remove them all together.
return matcher
.replaceAll("RANDOM-ISH_TEXT_TO_REPLACE")
.replaceAll("RANDOM-ISH_TEXT_TO_REPLACE\n?", "");
Copy link
Member

Choose a reason for hiding this comment

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

I don't have a better idea, but this looks.... odd...

Copy link
Author

Choose a reason for hiding this comment

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

I know but I could not find a better way either

}

@NonNull
public static String extendCategory(@NonNull String category) {
return category.replace("/", " / ");
Expand Down
6 changes: 6 additions & 0 deletions app/src/main/res/menu/menu_note_fragment.xml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,12 @@
android:orderInCategory="100"
android:title="@string/menu_change_category"
app:showAsAction="ifRoom" />
<item
android:id="@+id/menu_remove_checked_items"
android:icon="@drawable/ic_delete_grey600_24dp"
android:orderInCategory="100"
android:title="@string/menu_remove_checked_items"
app:showAsAction="ifRoom" />
<item
android:id="@+id/menu_share"
android:icon="@drawable/ic_share_white_24dp"
Expand Down
1 change: 1 addition & 0 deletions app/src/main/res/values-fr/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@
<string name="append_to_note">Ajouter à la note</string>
<string name="change_note_title">Modifier le titre de la note</string>
<string name="menu_edit_title">Modifier le titre</string>
<string name="menu_remove_checked_items">Supprimer éléments cochés</string>
<string name="simple_security">Sécurité</string>
<string name="appearance_and_behavior">Apparence et comportement</string>
<string name="simple_synchronization">Synchronisation</string>
Expand Down
1 change: 1 addition & 0 deletions app/src/main/res/values/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,7 @@
<string name="append_to_note">Append to note</string>
<string name="change_note_title">Change note title</string>
<string name="menu_edit_title">Edit title</string>
<string name="menu_remove_checked_items">Remove checked items</string>
<string name="simple_security">Security</string>
<string name="appearance_and_behavior">Appearance and behavior</string>
<string name="simple_synchronization">Synchronization</string>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,4 +99,40 @@ public void testGenerateNoteExcerpt_sdk_30() {
// content has markdown while titles markdown is already stripped
assertEquals("Title Bar", NoteUtil.generateNoteExcerpt("# Title\n- Title\n- Bar", "Title"));
}

@Test
public void testGetContentWithoutCheckedItems() {
String newLine = System.getProperty("line.separator");

String multilineWithCheckboxes = "- [ ] Line A"
.concat(newLine)
.concat("- [x] Line B")
.concat(newLine)
.concat("- [X] Line C")
.concat(newLine)
.concat(newLine)
.concat("* [x] Line D")
.concat(newLine)
.concat("* [ ] Line E")
.concat(newLine)
.concat("+ [x] Line F")
.concat(newLine)
.concat("+ [ ] Line G")
.concat(newLine)
.concat(" + [ ] G1")
.concat(newLine)
.concat(" + [x] G2");

String expected = "- [ ] Line A"
.concat(newLine)
.concat(newLine)
.concat("* [ ] Line E")
.concat(newLine)
.concat("+ [ ] Line G")
.concat(newLine)
.concat(" + [ ] G1")
.concat(newLine);
Copy link
Member

Choose a reason for hiding this comment

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

As stated above, there are very many cases of the commonmark specification not considered (and also not covered by the tests)... @AndyScherzinger and @tobiasKaminsky will have to decide how compliant is "compliant enough" 😉


assertEquals(expected, NoteUtil.getContentWithoutCheckedItems(multilineWithCheckboxes));
}
}
Loading