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

natural-time-delta sorting - remove the splitter key and split by full keyword #366

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

saveshodhan
Copy link
Contributor

  • Removed unwanted data
  • Corrected 'splitter' values passed for calculating time

-- Removed unwanted data
-- Corrected splitter names passed for calculating time
@saveshodhan
Copy link
Contributor Author

JSFiddle for this update:
https://jsfiddle.net/9vvd4fvr/1/

@DataTables
Copy link
Collaborator

Does that mean that you have to put the full text rather than just "1h 4m" for example?

@saveshodhan
Copy link
Contributor Author

I guess that was always the case.. Because the regular expression expects either hour or hours.. Reason was that 'hour' could be represented as h or hr or hour (likewise for plural cases). so i'd thought of matching common keywords (like hours or minutes, etc.)

In this commit, i have simply removed the splitter key (as it seemed redundant) and am sending the name itself as splitter..

We may go ahead with this change for now, i will have to think of how to implement shorter time formats as well..

What say?

@saveshodhan
Copy link
Contributor Author

Hello,

Hope you are doing well..!

I was going through the repo, and saw this commit waiting to merge.
I see a minor typo from my end that I am going to fix.

Meanwhile, regarding the query that you had asked in a comment above, just wanted to make sure that i was able to answer it in the subsequent comment, and that we are on the same page..

@saveshodhan
Copy link
Contributor Author

Probably what I meant above was that the string 2 months was earlier split using m, then I changed it to be split by month (JS would still give me the number 2 on 0th index). This is done because of an error - in the mappings, we have splitter as w for the last three elements (I guess "Refactoring" is a misleading name, will change it). So, if splitting by month or m is going to give same result, why have an extra splitter element in the dict?

Also, there is this ambiguity that m might mean months or minutes.

This ambiguity is also why I had decided to stick to full text rather than having short forms, which may answer your query

Does that mean that you have to put the full text rather than just "1h 4m" for example?

@saveshodhan saveshodhan changed the title Refactoring natural-time-delta sorting - remove the splitter key and split by full keyword Sep 27, 2018
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.

2 participants