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

Update dateformat requirement to latest (5.0.3) #1324

Closed
rcyeh opened this issue Jan 15, 2024 · 4 comments · Fixed by #1326
Closed

Update dateformat requirement to latest (5.0.3) #1324

rcyeh opened this issue Jan 15, 2024 · 4 comments · Fixed by #1326

Comments

@rcyeh
Copy link
Contributor

rcyeh commented Jan 15, 2024

Describe the bug

Installed foam 0.25.6 on VS Code 1.85.1. Set preference to make weekly notes with:

"foam.openDailyNote.filenameFormat": "yyyy/yy-WW"

(yyyy subdirectory, filename 2-digit year, hyphen, padded week number, based on the documentation at https://www.npmjs.com/package/dateformat for version 5.0.3, published two years ago)

Today is Monday, 15 January, the third week of the year. On opening the daily (weekly) note, the created filename is: 2024/24-33.md. I expected 2024/24-03.md. Thought, this is probably a regular expression problem in dateformat.

But looking in my ~/.vscode/extensions/foam.foam-vscode-0.25.6/package.json, there is a line:

"dependencies": {
  "dateformat": "^3.0.3",

and indeed, in that version of dateformat https://www.npmjs.com/package/dateformat/v/3.0.3 (six years ago), there is only support for the single W, not padded WW format.

Can we update the required dateformat version to 5.0.3?

Small Reproducible Example

No response

Steps to Reproduce the Bug or Issue

  1. Make the setting: "foam.openDailyNote.filenameFormat": "yyyy-WW",
  2. Open a daily note.

Expected behavior

I expected to be able to use any of the date formats specified in https://www.npmjs.com/package/dateformat, linked from the settings, but because the required dateformat version is 3.0.3, which does not support { DDD, DDDD, p, or WW}, when I use those formats in the "foam.openDailyNote.filenameFormat" setting, I get unexpected results.

In particular, in January, WW should return 03, but I get 33 instead.

Screenshots or Videos

No response

Operating System Version

ChromeOS

Visual Studio Code Version

1.85.1

Additional context

No response

@rcyeh
Copy link
Contributor Author

rcyeh commented Jan 15, 2024

I wonder how that version specification gets produced?

In this repository's package.json file, the dependencies section is empty

"dependencies": {}

But in the 0.25.6 package, I see the older version specified

"dateformat": "^3.0.3",

@riccardoferretti
Copy link
Collaborator

The dependency is defined in foam/packages/foam-vscode/package.json, and yes, it's completely fine to do the upgrade - happy to take it on?

@rcyeh
Copy link
Contributor Author

rcyeh commented Jan 17, 2024

Thanks! Created my first pull request ever; attached for your review.

rcyeh added a commit to rcyeh/foam that referenced this issue Jan 27, 2024
rcyeh added a commit to rcyeh/foam that referenced this issue Jan 27, 2024
rcyeh added a commit to rcyeh/foam that referenced this issue Jan 27, 2024
rcyeh added a commit to rcyeh/foam that referenced this issue Jan 27, 2024
rcyeh added a commit to rcyeh/foam that referenced this issue Jan 27, 2024
rcyeh added a commit to rcyeh/foam that referenced this issue Jan 27, 2024
rcyeh added a commit to rcyeh/foam that referenced this issue Feb 13, 2024
@rcyeh
Copy link
Contributor Author

rcyeh commented Feb 13, 2024

Since 4.5.1 supports the appropriate padded week codes and compiles with foam, we'll go to 4.5.1 instead of any higher ECMAscript version.

rcyeh added a commit to rcyeh/foam that referenced this issue Feb 14, 2024
Add jest to devDependencies in top-level package.json.
rcyeh added a commit to rcyeh/foam that referenced this issue Feb 14, 2024
rcyeh added a commit to rcyeh/foam that referenced this issue Feb 17, 2024
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 a pull request may close this issue.

2 participants