shabados / gurmukhi-utils

Utilities library for converting, analyzing, and testing Gurmukhi strings.
MIT License
30 stars 9 forks source link

feat: add functions to analyze pingal/matra/meter #168

Closed bhajneet closed 3 years ago

bhajneet commented 3 years ago

Summary of PR

There are schools of though on how to proof/read gurbani which count up the syllables in each line. Syllables are defined in issue #139. Further information on the background/practice of pingal/Pingala can be found here.

Tests for unexpected behavior

Time spent on PR

Between 8-12 hours total, mostly on research. Actual dev probably around two hours.

Linked issues

Fix #139

Reviewers

@Harjot1Singh @Sarabveer

Harjot1Singh commented 3 years ago

We’ve removed that rule deliberately, as having the parenthesis everywhere for consistency can also help make diffs less messy (I know it looks nicer for 1 paean functions though)

On 16 Oct 2020, at 16:52, ਸਹਜਪ੍ਰੀਤ ਸਿੰਘ notifications@github.com wrote:

 @saihaj commented on this pull request.

In lib/firstLetters.js:

@@ -29,13 +29,13 @@ const vowelFirstLetters = {

  • @example English first letters
  • firstLetters('sabad marai. so mar rahai; fir. marai na, doojee vaar |') // => sm.smr;f.mn,dv| */ -const firstLetters = line => line +const firstLetters = ( line ) => line I think this rule is missing and is causing that

"arrow-parens": [ "error", "as-needed", { "requireForBlockBody": false } ], — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

sarabveer commented 3 years ago

We’ve removed that rule deliberately, as having the parenthesis everywhere for consistency can also help make diffs less messy (I know it looks nicer for 1 paean functions though)

Then shouldn't it be done in a different PR?

bhajneet commented 3 years ago

If I do it in a separate PR I'll lose two days of work because harjot is busy as well. I needed the eslint updated for my work on the functions. Going to push all the changes you've both requested for a second review shortly.

Harjot1Singh commented 3 years ago

If I do it in a separate PR I'll lose two days of work because harjot is busy as well. I needed the eslint updated for my work on the functions. Going to push all the changes you've both requested for a second review shortly.

I'll probably rebase this PR, since there's a few unrelated bits in it. And squash any of the related bits together.

bhajneet commented 3 years ago

I believe everything except the regexes is resolved now. Will probably finish this PR over the weekend if I get the time. Thanks for all the suggestions so far, very interesting stuff!