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.

Sign up for GitHub

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

Jump to bottom

Supporting Duration for Datetime attributes #650

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

Supporting Duration for Datetime attributes #650

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6f0962b
Support the duration attribute
parulsingh23 Nov 4, 2020
6faaa2f
Support duration attribute
parulsingh23 Nov 4, 2020
297249c
Support duration attribute
parulsingh23 Nov 4, 2020
cf2c191
Updated duration to return an Array and follow code syntax of file
parulsingh23 Nov 24, 2020
07fb78b
Undo previous update to duration
parulsingh23 Nov 24, 2020
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
25 changes: 23 additions & 2 deletions dist/0.2.1/mavo.es5.js
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions src/elements.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -488,8 +488,14 @@ _.register({
"time": /^[H\d]{2}:[M\d]{2}/i,
"datetime-local": /^[Y\d]{4}-[M\d]{2}-[D\d]{2} [H\d]{2}:[Mi\d]{2}/i,
"date": /^[Y\d]{4}-[M\d]{2}-[D\d]{2}$/i,
"duration": /^P/,
"duration-normal": /(day[s]?|hour[s]?|minute[s]?|second[s]?)$/,
Copy link
Member

Choose a reason for hiding this comment

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

Ah, I see what you meant now. Yes, we'd need to add ms, weeks, months, years too.
One direction to explore: can we make this work for other languages beyond English? (look at the Intl object)

"duration-abbreviation": /(d|h|m|s)$/,
},
defaultFormats: {
"duration": name => "[getduration(".concat(name,"\"officialDurationString\")]"),
"duration-abbreviation": name => "[getduration(".concat(name,"\"abbreviatedDurationString\")]"),
"duration-normal": name => "[getduration(".concat(name,"\"normalDurationString\")]"),
Copy link
Member

Choose a reason for hiding this comment

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

Please use template strings for concatenation, which are more readable than .concat() method calls, and don't require escaping of double quotes. See the other values in this object literal for example.

"date": name => `[day(${name})] [month(${name}, 'shortname')] [year(${name})]`,
"month": name => `[month(${name}, 'name')] [year(${name})]`,
"time": name => `[hour(${name}, '00')]:[minute(${name}, '00')]`,
Expand Down
14 changes: 14 additions & 0 deletions src/functions.date.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,20 @@ $.extend(_, {
}, {multiValued: true}),

localTimezone: -(new Date()).getTimezoneOffset(),
getduration: $.extend(function (dur, format) {
Copy link
Member

Choose a reason for hiding this comment

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

It's unclear what the purpose of getduration() is, or how reusable it is. It seems to take a formatted duration string, and return a different formatted duration string. I would suggest decoupling it into a function that parses a duration string and returns a number, and then we already have a function that takes a number and converts it to a formatted duration string (duration()).

A few more minor things:

  • Avoid abbreviated names like dur
  • inconsistent naming, camelCase in the line above, lowercase concatenated in this one. In general, JS tends to use camelCase, and it's usually good to follow the naming conventions of the host language.
  • Object.assign() should work fine instead of $.extend() here, the main difference being that the former executes accessors and assigns the result, whereas the latter transfers the accessors. When there are no accessors, Object.assign() is preferable as it's native to the platform, and thus faster.

if(format === "officialDurationString") {durationRegEx = /P(?:([.,\d]+)D)?(?:T(?:([.,\d]+)H)?(?:([.,\d]+)M)?(?:([.,\d]+)S)?)?/;}
Copy link
Member

Choose a reason for hiding this comment

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

Please install ESLint + an ESLint plugin for your editor so you can get inline warnings when the coding style of the project is not followed.

if(format === "abbreviatedDurationString") {durationRegEx = /[\s]*(?:([.,\d]+)d)?[\s]*(?:([.,\d]+)h)?[\s]*(?:([.,\d]+)m)?[\s]*(?:([.,\d]+)s)?[\s]*/;}
if(format === "normalDurationString") {durationRegEx = /[\s]*(?:([.,\d]+)[ ]?day[s]?)?[\s]*(?:([.,\d]+)[ ]?hour[s]?)?[\s]*(?:([.,\d]+)[ ]?minute[s]?)?[\s]*(?:([.,\d]+)[ ]?second[s]?)?[\s]*/;}
var matches = dur.match(durationRegEx);
Copy link
Member

Choose a reason for hiding this comment

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

So this is an interesting violation of DRY. Interesting because it's less obvious than simply duplicated code, but there is still more than one sources of truth. The dur parameter already contains the information about what format the parameter is in. These formats are mutually exclusive, and thus, there is no ambiguity that requires such a hint. Providing the format as a second parameter is done for developer convenience, not user convenience. It's better for your code to determine the format automatically by trying to parse as the 1st format, and failing that the 2nd format and so on.

var dur_days = matches[1] === undefined ? 0 : matches[1];
var dur_hours = matches[2] === undefined ? 0 : matches[2];
var dur_minutes = matches[3] === undefined ? 0 : matches[3];
var dur_seconds = matches[4] === undefined ? 0 : matches[4];

var dur_num = dur_days*24*60*60*1000+dur_hours*60*60*1000+dur_minutes*60*1000+dur_seconds*1000;

return this.duration(dur_num);
}),
});

_.msTo = (what, ms) => Math.floor(Math.abs(ms) / (s[what] * 1000)) || 0;
Expand Down