core: adjust field collector api, fix #94

packages/koishi-core/src/command.ts

@@ -102,7 +102,10 @@ type Extend<O extends {}, K extends string, T> = {
   [P in K | keyof O]: (P extends keyof O ? O[P] : unknown) & (P extends K ? T : unknown)
 }
 
-type ArgvInferred<T> = Iterable<T> | ((argv: ParsedArgv, fields: Set<T>) => Iterable<T>)
+export type FieldCollector<T extends TableType, K = keyof Tables[T], O extends {} = {}> =
+  | Iterable<K>
+  | ((argv: ParsedArgv<never, never, O>, fields: Set<keyof Tables[T]>) => void)
+
 export type CommandAction<U extends User.Field = never, G extends Group.Field = never, O = {}> =
   (this: Command<U, G>, config: ParsedArgv<U, G, O>, ...args: string[]) => void | string | Promise<void | string>
 
@@ -117,36 +120,37 @@ export class Command<U extends User.Field = never, G extends Group.Field = never
 
   private _optionNameMap: Record<string, CommandOption> = {}
   private _optionSymbolMap: Record<string, CommandOption> = {}
-  private _userFields: ArgvInferred<User.Field>[] = []
-  private _groupFields: ArgvInferred<Group.Field>[] = []
+  private _userFields: FieldCollector<'user'>[] = []
+  private _groupFields: FieldCollector<'group'>[] = []
 
   _action?: CommandAction<U, G>
 
   static defaultConfig: CommandConfig = {}
   static defaultOptionConfig: OptionConfig = {}
 
-  private static _userFields: ArgvInferred<User.Field>[] = []
-  private static _groupFields: ArgvInferred<Group.Field>[] = []
+  private static _userFields: FieldCollector<'user'>[] = []
+  private static _groupFields: FieldCollector<'group'>[] = []
 
-  static userFields(fields: ArgvInferred<User.Field>) {
+  static userFields(fields: FieldCollector<'user'>) {
     this._userFields.push(fields)
     return this
   }
 
-  static groupFields(fields: ArgvInferred<Group.Field>) {
+  static groupFields(fields: FieldCollector<'group'>) {
     this._groupFields.push(fields)
     return this
   }
 
   static collect<T extends TableType>(argv: ParsedArgv, key: T, fields = new Set<keyof Tables[T]>()) {
     if (!argv) return
-    const values: ArgvInferred<keyof Tables[T]>[] = [
+    const values: FieldCollector<T>[] = [
       ...this[`_${key}Fields`],
       ...argv.command[`_${key}Fields`],
     ]
-    for (let value of values) {
+    for (const value of values) {
       if (typeof value === 'function') {
-        value = value(argv, fields)
+        value(argv, fields)
+        continue
       }
       for (const field of value) {
         fields.add(field)
@@ -183,18 +187,14 @@ export class Command<U extends User.Field = never, G extends Group.Field = never
     return `Command <${this.name}>`
   }
 
-  userFields<T extends User.Field = never>(fields: Iterable<T>): Command<U | T, G, O>
-  userFields<T extends User.Field = never>(fields: (argv: ParsedArgv<never, never, O>, fields: Set<User.Field>) => Iterable<T>): Command<U | T, G, O>
-  userFields(fields: ArgvInferred<User.Field>) {
+  userFields<T extends User.Field = never>(fields: FieldCollector<'user', T, O>): Command<U | T, G, O> {
     this._userFields.push(fields)
-    return this
+    return this as any
   }
 
-  groupFields<T extends Group.Field = never>(fields: Iterable<T>): Command<U, G | T, O>
-  groupFields<T extends Group.Field = never>(fields: (argv: ParsedArgv<never, never, O>, fields: Set<Group.Field>) => Iterable<T>): Command<U, G | T, O>
-  groupFields(fields: ArgvInferred<Group.Field>) {
+  groupFields<T extends Group.Field = never>(fields: FieldCollector<'group', T, O>): Command<U, G | T, O> {
     this._groupFields.push(fields)
-    return this
+    return this as any
   }
 
   alias(...names: string[]) {

packages/koishi-core/src/plugins/help.ts

@@ -1,6 +1,6 @@
 import { getUsage, getUsageName, ValidationField } from './validate'
 import { User, Group, TableType, Tables } from '../database'
-import { Command, ParsedArgv } from '../command'
+import { Command, FieldCollector, ParsedArgv } from '../command'
 import { Session } from '../session'
 import { App } from '../app'
 import { Message } from './message'
@@ -59,17 +59,15 @@ export default function apply(app: App) {
     return ''
   })
 
-  function createCollector<T extends TableType>(key: T) {
-    return function* (argv: ParsedArgv, fields: Set<keyof Tables[T]>) {
-      const { args: [name] } = argv
-      const command = app._commandMap[name] || app._shortcutMap[name]
-      if (!command) return
-      yield* Command.collect({ ...argv, args: [], options: { help: true } }, key, fields)
-    }
+  const createCollector = <T extends TableType>(key: T): FieldCollector<T> => (argv, fields) => {
+    const { args: [name] } = argv
+    const command = app._commandMap[name] || app._shortcutMap[name]
+    if (!command) return
+    Command.collect({ ...argv, command, args: [], options: { help: true } }, key, fields)
   }
 
   app.command('help [command]', '显示帮助信息', { authority: 0 })
-    .userFields(['authority'])
+    .userFields<ValidationField>(['authority'])
     .userFields(createCollector('user'))
     .groupFields(createCollector('group'))
     .shortcut('帮助', { fuzzy: true })
@@ -98,6 +96,7 @@ export default function apply(app: App) {
           },
         })
       }
+
       return showHelp(command, session, options)
     })
 }

packages/koishi-core/src/plugins/validate.ts

@@ -59,7 +59,7 @@ Object.assign(Command.defaultOptionConfig, {
   authority: 0,
 })
 
-Command.userFields(function* ({ command, options = {} }, fields) {
+Command.userFields(({ command, options = {} }, fields) => {
   const { maxUsage, minInterval, authority } = command.config
   let shouldFetchAuthority = !fields.has('authority') && authority > 0
   let shouldFetchUsage = !!(maxUsage || minInterval)
@@ -69,10 +69,10 @@ Command.userFields(function* ({ command, options = {} }, fields) {
       if (notUsage) shouldFetchUsage = false
     }
   }
-  if (shouldFetchAuthority) yield 'authority'
+  if (shouldFetchAuthority) fields.add('authority')
   if (shouldFetchUsage) {
-    if (maxUsage) yield 'usage'
-    if (minInterval) yield 'timers'
+    if (maxUsage) fields.add('usage')
+    if (minInterval) fields.add('timers')
   }
 })
 

packages/koishi-core/tests/command.spec.ts

@@ -1,12 +1,11 @@
 import { App } from 'koishi-test-utils'
+import { inspect } from 'util'
 import { expect } from 'chai'
 import '@shigma/chai-extended'
 
-let app: App
-
 describe('Command API', () => {
   describe('Register Commands', () => {
-    before(() => app = new App())
+    const app = new App()
 
     it('constructor checks', () => {
       expect(() => app.command('')).to.throw()
@@ -26,6 +25,10 @@ describe('Command API', () => {
       expect(app._commandMap.c.context).to.equal(ctx2)
     })
 
+    it('custom inspect', () => {
+      expect(inspect(app.command('a'))).to.equal('Command <a>')
+    })
+
     it('modify commands', () => {
       const d1 = app.command('d', 'foo', { authority: 1 })
       expect(app._commandMap.d.config.description).to.equal('foo')
@@ -63,6 +66,7 @@ describe('Command API', () => {
   })
 
   describe('Register Subcommands', () => {
+    let app: App
     beforeEach(() => app = new App())
 
     it('command.subcommand', () => {
@@ -123,4 +127,28 @@ describe('Command API', () => {
       expect(() => app.command('a/d')).not.to.throw()
     })
   })
+
+  describe('Dispose Commands', () => {
+    const app = new App()
+    const foo = app.command('foo')
+    const bar = foo.subcommand('bar')
+    const test = bar.subcommand('test')
+    bar.alias('baz').shortcut('1')
+    test.alias('it').shortcut('2')
+
+    it('before dispose', () => {
+      // don't forget help
+      expect(app._commands).to.have.length(4)
+      expect(app._shortcuts).to.have.length(3)
+      expect(foo.children).to.have.length(1)
+    })
+
+    it('after dispose', () => {
+      bar.dispose()
+      // don't forget help
+      expect(app._commands).to.have.length(2)
+      expect(app._shortcuts).to.have.length(1)
+      expect(foo.children).to.have.length(0)
+    })
+  })
 })

packages/koishi-core/tests/parser.spec.ts

@@ -1,6 +1,5 @@
 import { Command } from 'koishi-core'
 import { App } from 'koishi-test-utils'
-import { inspect } from 'util'
 import { expect } from 'chai'
 import '@shigma/chai-extended'
 
@@ -10,28 +9,23 @@ let cmd: Command
 
 describe('Parser API', () => {
   describe('Basic Support', () => {
-    it('register', () => {
-      // there is a built-in help command
-      expect(app._commands).to.have.length(1)
-
-      cmd = app.command('cmd1 <foo> [...bar]')
-      expect(app._commands).to.have.length(2)
-    })
-
-    it('inspect', () => {
-      expect(inspect(cmd)).to.equal('Command <cmd1>')
-    })
-
     it('parse arguments', () => {
+      cmd = app.command('cmd1 <foo> [...bar]')
       expect(cmd.parse('')).to.have.shape({ args: [] })
       expect(cmd.parse('a')).to.have.shape({ args: ['a'] })
       expect(cmd.parse('a b')).to.have.shape({ args: ['a', 'b'] })
       expect(cmd.parse('a b c')).to.have.shape({ args: ['a', 'b', 'c'] })
     })
 
-    it('dispose', () => {
-      cmd.dispose()
-      expect(app._commands).to.have.length(1)
+    it('stringify arguments', () => {
+      cmd = app.command('cmd4')
+      cmd.option('alpha', '-a <val>')
+      cmd.option('beta', '-b')
+      expect(cmd.stringify(['foo', 'bar'], {})).to.equal('cmd4 foo bar')
+      expect(cmd.stringify([], { alpha: 2 })).to.equal('cmd4 --alpha 2')
+      expect(cmd.stringify([], { alpha: ' ' })).to.equal('cmd4 --alpha " "')
+      expect(cmd.stringify([], { beta: true })).to.equal('cmd4 --beta')
+      expect(cmd.stringify([], { beta: false })).to.equal('cmd4 --no-beta')
     })
   })
 
@@ -140,17 +134,4 @@ describe('Parser API', () => {
       expect(cmd.parse('-- "foo;bar";baz', ';')).to.have.shape({ options: { rest: 'foo;bar' }, rest: ';baz' })
     })
   })
-
-  describe('Stringify Argv', () => {
-    it('basic support', () => {
-      cmd = app.command('cmd4')
-      cmd.option('alpha', '-a <val>')
-      cmd.option('beta', '-b')
-      expect(cmd.stringify(['foo', 'bar'], {})).to.equal('cmd4 foo bar')
-      expect(cmd.stringify([], { alpha: 2 })).to.equal('cmd4 --alpha 2')
-      expect(cmd.stringify([], { alpha: ' ' })).to.equal('cmd4 --alpha " "')
-      expect(cmd.stringify([], { beta: true })).to.equal('cmd4 --beta')
-      expect(cmd.stringify([], { beta: false })).to.equal('cmd4 --no-beta')
-    })
-  })
 })