-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
edited notes: recognize TODAY keyword #7704
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
base: main
Are you sure you want to change the base?
edited notes: recognize TODAY keyword #7704
Conversation
Summary of ChangesHello @contributor, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly improves the usability of date-based notes by enabling dynamic date keyword recognition within #dateNote labels. Users can now easily reference "today," "this month," or "last year" notes, along with numerical offsets, streamlining the process of creating and accessing recurring or time-sensitive information. This change centralizes date-related note access and enhances the overall navigation experience. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a great new feature allowing the use of date keywords like TODAY, MONTH, and YEAR for date notes. The implementation is well-structured, involving a clean refactoring of existing logic into a new edited-notes.ts file with its own comprehensive test suite. My review includes a few suggestions to improve robustness, address a functional limitation, and enhance code quality. Overall, this is a solid contribution.
this allows to return additional field along with notes, for example, a flag to indicate if response was truncated by limit
6690220 to
7c70101
Compare
7c70101 to
bcff929
Compare
|
/gemini review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a great new feature for recognizing date keywords like TODAY, MONTH, and YEAR. The implementation is well-structured, with the logic correctly extracted into a new edited-notes.ts file and accompanied by a comprehensive test suite.
My review includes a few suggestions to improve robustness and readability:
- Tightening the validation for date string prefixes.
- Improving variable naming to avoid shadowing in the React component.
- A suggestion to enhance the new test suite.
- Removing some leftover commented-out code.
Overall, this is a solid contribution.
|
Ready for review. Command to run tests: Test results: |
eliandoran
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here I have a few questions and remarks:
This only affects the "Edited Notes" functionality?
If so, do we really need this?
As I user I find it a bit strange that I have a "Today" note that I can write into but it has no other special meaning, apart from the list of edited notes in the ribbon.
Can't this be replicated with already existing functionality such as Saved Search. If not, can we alter that instead? This would have the benefit of actually displaying the results inside the note content instead of just the ribbon.
The other remark is that the documentation also needs to be updated to reflect this new functionality.
“Today” is a name you chose yourself. You can name it whatever you like (e.g. “Edited Today”) or not create it at all. Yes, it's a kind of a “pseudo” note. But regular day notes is “pseudo” notes in my use case too. I never write in them. They only exist for Edited Notes functionality - so I can view what was edited this day or this month etc. Most often I need this for today or yesterday notes. And for this, I don't want to dive into full-blown calendar tree to view those (and pollute the tree by auto-creating concrete date notes that will be used once - I even have "auto-expire/auto-delete" task in backlog to remove them, along with search history tree notes and other temporal notes).
I tried to use Saved Search for this purpose for years - it didn't work:
In contrast, “Edited Today” is single note in predicted place in the tree (and with assigned keyboard shortcut). From the edited notes ribbon you open a note you interested in. (My edited notes has custom styles and looks different from default - the ribbon tab is almost “full screen”, and displays one note link per line, not a heap of links - this is not in the PR).
I will add the docs if this going to be merged |
to be compatible with existing search syntax
I do have a similar feature. It creates temporal note called “Edited notes [date]” with children which are clones of edited notes. It is independent/complimentary to “Edited today”or any note that has “Edited notes” ribbon. This is experimental feature, I don't use it as often as simple “Edited today” note from this PR (which became my “recent inbox cache” (for general quick access) and “inbox memory” (for quick access to notes that moved out of inbox to their (deep-in-tree) proper place) |

This allows to use date keywords in
#dateNotelabel to create “permanent” date notes like today (#dateNote=TODAY), yesterday (#dateNote=TODAY-1) and so on.This way you can see which notes have been edited on recent days without using full calendar tree or scroll via Recent Changes modal window.
Example use case: replace keyboard shortcut “Open Today's Journal Note” (#7472) with note label
#keyboardShortcut='key-combo'to have today note in single predictable place instead of jumping/expanding calendar tree.MONTH and YEAR keywords are supported too, you can create note like:
MONTH and YEAR are seem to be less useful than TODAY. Edited Notes for those are likely to be capped by current 50 note limit in edited-noted sql query (I have not changed the limit in this PR, but for month/year notes it certainly should be higher).
The logic is written to be extendable with “date range” queries in the future. But this will require an additional label. Single label #dateNote is not enough for that.