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

Fix: getDeepPropFromObj does not work #82

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

anseki
Copy link

@anseki anseki commented Jul 18, 2015

A following code in getDeepPropFromObj function in preprocess.js doesn't work:

  propPath.replace(/\[([^\]+?])\]/g, '.$1');

The .replace() method returns a replaced string, and it doesn't change a original string (i.e. propPath). The above code drops a replaced string.
The replaced string must be received:

  propPath = propPath.replace(/\[([^\]+?])\]/g, '.$1');

A current code doesn't catch the properties that is named 2 or more length when VAR[PROP] style is specified.
For example:

var src =
  'VAR.A: <!-- @echo VAR.A -->\n' +
  'VAR.B: <!-- @echo VAR.B -->\n' +
  'VAR.AA: <!-- @echo VAR.AA -->\n' +
  'VAR.BB: <!-- @echo VAR.BB -->\n' +
  'VAR[A]: <!-- @echo VAR[A] -->\n' +
  'VAR[B]: <!-- @echo VAR[B] -->\n' +
  'VAR[AA]: <!-- @echo VAR[AA] -->\n' +
  'VAR[BB]: <!-- @echo VAR[BB] -->\n'
  ;

console.log(
  require('preprocess').preprocess(src, {
    VAR: {
      A: 'var.a',
      B: 'var.b',
      AA: 'var.aa',
      BB: 'var.bb'
    }
  })
);

Result:

VAR.A: var.a
VAR.B: var.b
VAR.AA: var.aa
VAR.BB: var.bb
VAR[A]: var.a
VAR[B]: var.b
VAR[AA]: undefined
VAR[BB]: undefined

A regexp in current code:

/\[([^\]+?])\]/g

This matches one character, except ], +, and ?.
I think that +? should have been a pattern not literal:

/\[([^\]]+?)\]/g

And, [^\]] is unnecessary because ? works:

/\[(.+?)\]/g

Result by the above:

VAR.A: var.a
VAR.B: var.b
VAR.AA: var.aa
VAR.BB: var.bb
VAR[A]: var.a
VAR[B]: var.b
VAR[AA]: var.aa
VAR[BB]: var.bb

A current code doesn't discern a property name via VAR.PROP and VAR[PROP], and it splits also a property name of VAR[PROP].
For example:

var src =
  'VAR.A.B: <!-- @echo VAR.A.B -->\n' +
  'VAR[A.B]: <!-- @echo VAR[A.B] -->\n'
  ;

console.log(
  require('preprocess').preprocess(src, {
    VAR: {
      A: {B: 'var.a.b'},
      'A.B' : 'var[a.b]'
    }
  })
);

Result:

VAR.A.B: var.a.b
VAR[A.B]: var.a.b

This PR discerns those.
Result by fixed code:

VAR.A.B: var.a.b
VAR[A.B]: var[a.b]

This fixed code discerns also a name like VAR[AA].B[CC.DD][E] correctly.

@BendingBender
Copy link
Contributor

Thank you for your PR. I've implemented it quick-and-dirty style and didn't test it. This was not meant to be part of officially documented functionality just yet.

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