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

Created and updated times #56

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open

Created and updated times #56

wants to merge 13 commits into from

Conversation

mlshapiro
Copy link

@mlshapiro mlshapiro commented Dec 23, 2023

Add created/updated timestamps to block delimiter (#30)

New delimiter format with -c for created time and -u for updated time with ISO strings following (YYYY-MM-DDTHH:mm:ssZ)

\n∞∞∞(${languageTokensMatcher})(-a)?(-c${timeMatcher})?(-u${timeMatcher})?\n

Presents the updated time in the status bar:

image

@mlshapiro mlshapiro changed the title [DRAFT] Created and updated times (#30) [DRAFT] Created and updated times Dec 23, 2023
@heyman
Copy link
Owner

heyman commented Dec 23, 2023

If this is headed in the right direct, i can keep going

I think so 👍! It was a while since I messed with the Lezer stuff, and the Lezer code in Heynote is the full extent of my experience with it, so I'm certainly not an expert when it comes to the grammar syntax.

@heyman
Copy link
Owner

heyman commented Dec 24, 2023

A thought that popped up in my mind when thinking about this.. I think it would be a good idea to investigate how we could implement the actual updating of the timestamps in the Codemirror text buffer. I'm guessing that this should be done with some kind of transactionFilter, but the text in the reference manual seems to indicate that it might not be a very good idea. So it's probably smart to investigate that to make sure it's not a showstopper due to performance issues or similar.

@heyman
Copy link
Owner

heyman commented Dec 25, 2023

We would only need a transactionFilter to update the update times with every change. So it even if it turns out that it's a showstopper, it shouldn't prevent us from adding creation time.

@mlshapiro mlshapiro marked this pull request as draft December 25, 2023 03:22
@mlshapiro mlshapiro changed the title [DRAFT] Created and updated times Created and updated times Dec 28, 2023
@mlshapiro mlshapiro marked this pull request as ready for review December 28, 2023 05:37
@@ -131,6 +135,8 @@
:theme="theme"
:systemTheme="systemTheme"
:allowBetaVersions="settings.allowBetaVersions"
:createdTime="createdTime"
Copy link
Author

Choose a reason for hiding this comment

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

This doesn't currently show the createdTime, so these props could be removed.

If you want to show the createdTime somewhere, then the prop could be kept

<div class="status-block line-number" v-if="updatedTime !== ''">
Updated <span class="num">{{ updatedTime }}</span>
<!-- Show the created time as well -- too verbose -->
<!-- <span v-if="createdTime !== updatedTime">&nbsp;&nbsp;(Created {{ createdTime }})</span> -->
Copy link
Author

Choose a reason for hiding this comment

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

This looked too busy to me. Perhaps the better UI would be to click on the udpated time to see the created time? That wouldn't be clear either though.

Feel free to adjust this as you see it


const languageTokensMatcher = LANGUAGES.map(l => l.token).join("|")
Copy link
Author

Choose a reason for hiding this comment

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

I had to copy this snippet over a few times - perhaps this goes in languges.js and can be imported?

if (state.doc.sliceString(block.delimiter.from, block.delimiter.to).match(delimRegex)) {
//console.log("changing language to", language)
const createdTimeStr = block.time.created || ""
const updatedTimeStr = block.time.updated ? newUpdatedTime() : ""
Copy link
Author

Choose a reason for hiding this comment

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

This isn't explicitly needed - this will get updated by the transactionFilter, but it doesn't hurt to add it since the other function has the transactionFilter has the debounce

@@ -23,7 +24,7 @@ export const noteContent = new ExternalTokenizer((input) => {
// so we don't need to check for the rest of the token
if (current === FIRST_TOKEN_CHAR && next === SECOND_TOKEN_CHAR) {
let potentialLang = "";
for (let i=0; i<18; i++) {
for (let i=0; i<62; i++) {
Copy link
Author

Choose a reason for hiding this comment

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

Note I added the 40 characters of the times to this.

@mlshapiro
Copy link
Author

mlshapiro commented Jan 2, 2024

@heyman this is "ready to go" but I'm having trouble solving two (known) bugs:

  1. When I "Select All" (twice) to get the whole scratch pad, and then "delete", I'm left with 2 blocks, not one. I can't fingure out why the defaultSelectAll doesn't actually select all the buffer text.
  2. I can't get certain language-detection.spec.js tests to pass. When I debug the tests, the .pressSequentially(... command types in the text and then ends up in an infinite loop of creating new blocks. I can't figure out where this is coming from and whether the bug is in the test or application side.

Any insight on either?

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.

None yet

2 participants