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

refactor: centralize lg parsing logic to 'shared' lib #1663

Merged
merged 46 commits into from
Dec 2, 2019

Conversation

yeze322
Copy link
Contributor

@yeze322 yeze322 commented Nov 26, 2019

Description

Closes #1308.
Closes #1659.

Merges all LG related logic to one module in '@bfc/shared'. Prepares for the upcoming lg syntax update.

Where are we parsing lg strings?

  • In 'visual -> ObiEditor.ts -> line 52, 92', we parse lg string to extract lg name and node id to perform lg copy.
  • In 'visual -> utils -> hooks.ts', we parse lg string to extract lg name (and then invoke lgApi to update template)
  • In 'client -> lgUtils.ts', we parse lg string to extract lg name
  • In 'packages/lib -> indexers -> dialogIndexer.ts', we parse lg string to extract lg name
  • In 'form -> TableField -> line 100', extract lg name out of string (should be coverd in another PR which focuses on shellApi refinement)
  • In 'form -> LgEditorWidget.tsx -> line 35~52, line 102', construct a lg id.
  • In 'form -> LgEditorWidget.tsx' -> line 18~22, handle lgText.

How we understand LG strings

  1. LgText
  {
      "activity": "....", // LgText
      "prompt": "....", // LgText
      "invalidPrompt": "..." // LgText
  }

  2. LgTemplateRef
  '[bfdactivity-1234]'
  '[greeting]'
  '[greeting(1)]'

  3. LgTemplateName
  'bfdactivity-1234' in '[bfdactivity-1234]'
  'greeting' in '[greeting(1)]'

  4. LgMetaData (Composer-only, can be converted to LgTemplateName)
  'bfdactivity-1234' => { type: 'activity', designerId: '1234' } // LgMetaData
  'greeting' => NO meta data since its not created by Composer

Task Item

Closes #1308

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Code refactor (non-breaking change which improve code quality, clean up, add tests, etc)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Doc update (document update)

Checklist

  • I have added tests that prove my fix is effective or that my feature works
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have functionally tested my change

Screenshots

Please include screenshots or gifs if your PR include UX changes.

@yeze322 yeze322 changed the title implement basic lg apis Organize lg parsing logic to 'shared' lib (#1659) Nov 26, 2019
@yeze322 yeze322 changed the title Organize lg parsing logic to 'shared' lib (#1659) chore: Organize lg parsing logic to 'shared' lib (#1659) Nov 27, 2019
@yeze322 yeze322 changed the title chore: Organize lg parsing logic to 'shared' lib (#1659) refactor: Organize lg parsing logic to 'shared' lib (#1659) Nov 27, 2019
@yeze322 yeze322 marked this pull request as ready for review November 27, 2019 10:51
@yeze322 yeze322 requested a review from zhixzhan November 27, 2019 10:54
if (matches && matches.length === 2) return matches[1];
return '';
const lgTemplateRef = LgTemplateRef.parse(x);
return lgTemplateRef ? lgTemplateRef.name : '';
Copy link
Contributor Author

@yeze322 yeze322 Nov 27, 2019

Choose a reason for hiding this comment

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

We can leverage the 'optional chaining' feature in ts 3.7. LgTemplateRef.parse(x)?.name

Copy link
Contributor

Choose a reason for hiding this comment

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

const newLgName = lgMetadata.toLgTemplateName();
const newLgTemplateRefString = lgMetadata.toLgTemplateRefString();

await copyLgTemplate('common', lgTemplateRef.name, newLgName);
Copy link
Contributor Author

@yeze322 yeze322 Nov 27, 2019

Choose a reason for hiding this comment

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

copyLgTemplate will be defined as copyLgText in my following PR. Targets to hide LG concepts from visual editor (and Form Editor), will inject a synthesized lg api from Shell level.

@@ -49,12 +49,10 @@ export const ObiEditor: FC<ObiEditorProps> = ({
);

const deleteLgTemplates = (lgTemplates: string[]) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

deleteLgTemplates will be defined as deleteLgText. See my next comments on copyLgTemplate

@boydc2014
Copy link
Contributor

LGTM, but let's wait a little bit until @cwhitten and @a-b-r-o-w-n can have a chance to look at. Also, if LSP is also good to go, let's merge LSP first.

boydc2014
boydc2014 previously approved these changes Nov 28, 2019
@a-b-r-o-w-n a-b-r-o-w-n changed the title refactor: centralize lg parsing logic to 'shared' lib (#1659) refactor: centralize lg parsing logic to 'shared' lib Dec 2, 2019
@cwhitten cwhitten merged commit 12f6c62 into microsoft:master Dec 2, 2019
@a-b-r-o-w-n a-b-r-o-w-n mentioned this pull request Dec 11, 2019
@a-b-r-o-w-n a-b-r-o-w-n mentioned this pull request Dec 11, 2019
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.

4 participants