Skip to content

Conversation

aaronpoweruser
Copy link
Collaborator

No description provided.

If lines start to wrap the formating breaks.
Typos
Resize images
@aaronpoweruser aaronpoweruser requested a review from jgclark April 24, 2024 01:18
@jgclark
Copy link
Collaborator

jgclark commented Apr 24, 2024

I'm happy with the tidy up and documentation changes.
But I'm not fully understanding the change to maxTermLen. If I understand the code right, then this will mean having a hugely long (>50 character) hashtags or mentions will stop padding, and make all of the progress update essentially left-aligned without padding. This will surely look ugly?
Have you considered the alternative which is to internally truncate the very long hashtag/mentions? Eg. "#thisIsSoLongThat...APR" instead of "#thisIsSoLongThatItsCausingWritingOfAPR".

(Longer term I want to be able to generate and insert proper graphical sparkline graphs here instead of the 'ASCII Art', but the necessary support isn't really available in NP yet.)

@aaronpoweruser
Copy link
Collaborator Author

Your right truncating will look better. Ill make a new ps.

@jgclark
Copy link
Collaborator

jgclark commented Apr 24, 2024

Thanks for checking with me on this.
Feel free to release the version with shortening without checking in with me first.

@aaronpoweruser
Copy link
Collaborator Author

Checklist padding fix

There is some weirdness with rendered text link links but this is good enough for now.

@dwertheimer
Copy link
Collaborator

Ah yes, nice catch. we have seen this issue in the main app also. I would suggest filtering out links and replacing with the text in the brackets before truncating. @jgclark do we have a helper for this? We certainly have code which could be helpful when we convert markdown links to HTML links.

@dwertheimer
Copy link
Collaborator

@aaronpoweruser ^^

Signed-off-by: Aaron Gascoigne <[email protected]>
@aaronpoweruser
Copy link
Collaborator Author

Done, while I was there added template render shoutout @mikeerickson for good code.

Code looks right however I am running into a bug I have seen before were "await" is not honored and code execution continues before the promise is completed. I thought it was related to date returned by Np but it seems its on the
Node side as this code is pure JS and does not use the Np api.

2024-04-24 18:03:42 | INFO  | gatherOccurrences :: Gathered CompletedChecklistItems data in 19ms
2024-04-24 18:03:42 ❗️ERROR❗️ generateProgressUpdate :: this should be after the await
2024-04-24 18:03:42 | INFO  | makeProgressUpdate :: Appended progress update for this week to current note
2024-04-24 18:03:42 ❗️ERROR❗️ generateProgressUpdate :: rendered term:  💭 Plan day [review 2024-W17](noteplan://x-callback-url/openNote?noteTitle=2024-W17)


await occObjs.forEach(async (m) => {
if (m.term.includes('<%') && m.term.includes('%>')) {
m.term = await NPTemplating.render(m.term)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this is returning late

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 what you're trying to achieve here when you say simply "render templates".
Nor do I know the Templating code, and as it's not well documented, it would take me a very long time to figure out. So I can't help. Do you want to back out this part of the PR for now?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks pretty cool to me. You could include template code in the title. Sounds like it could be useful. @aaronpoweruser What's your use case? And you tested it I assume.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@aaronpoweruser Can you pls provide some test examples that show how your new code is used, especially the new template parts? I don't see any mention of the template strings in the README. I may have missed it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure, I can update the documentation, this was a for me feature. As I dynamically generate a link to my weekly note.

@jgclark
Copy link
Collaborator

jgclark commented Jun 4, 2024

Code looks right however I am running into a bug I have seen before were "await" is not honored and code execution continues before the promise is completed.

@dwertheimer I wonder if this is related to the issue I'm seeing where commands in templates are returning [object Promise] for me?

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.

3 participants