From 1cfbf4fb663411c380be581fd4f1535b4cf7dd92 Mon Sep 17 00:00:00 2001 From: Bryan Ashby Date: Wed, 23 Aug 2023 18:06:48 -0600 Subject: [PATCH] Update MenuFlags to work as expected * 'popParent' has been removed * 'noHistory' now works as expected * Mods that explicitly want noHistory can state such in their constructor() --- WHATSNEW.md | 1 + core/file_area_list.js | 6 +- core/file_base_area_select.js | 6 +- core/file_base_download_manager.js | 6 +- core/file_base_search.js | 1 - core/file_base_user_list_export.js | 5 +- core/file_base_web_download_manager.js | 6 +- core/menu_module.js | 22 ++++++ core/menu_stack.js | 82 ++++++++--------------- core/message_base_search.js | 2 - core/msg_area_list.js | 7 +- core/msg_conf_list.js | 7 +- core/my_messages.js | 5 +- core/show_art.js | 2 +- core/upload.js | 4 +- docs/_docs/admin/oputil.md | 2 +- docs/_docs/configuration/menu-hjson.md | 10 +-- docs/_docs/modding/show-art.md | 1 - misc/menu_templates/file_base.in.hjson | 8 +-- misc/menu_templates/message_base.in.hjson | 4 +- 20 files changed, 96 insertions(+), 91 deletions(-) diff --git a/WHATSNEW.md b/WHATSNEW.md index 708653f0..060bf502 100644 --- a/WHATSNEW.md +++ b/WHATSNEW.md @@ -8,6 +8,7 @@ This document attempts to track **major** changes and additions in ENiGMA½. For * Routes for the file base now default to `/_f/` prefixed instead of just `/f/`. If `/f/` is in your `config.hjson` you are encouraged to update it! * Finally, the system will search for `index.html` and `index.htm` in that order, if another suitable route cannot be established. * CombatNet has shut down, so the module (`combatnet.js`) has been removed. +* The Menu Flag `popParent` has been removed and `noHistory` has been updated to work as expected. In general things should "Just Work", but check your `menu.hjson` entries if you see menu stack issues. ## 0.0.13-beta * **Note for contributors**: ENiGMA has switched to [Prettier](https://prettier.io) for formatting/style. Please see [CONTRIBUTING](CONTRIBUTING.md) and the Prettier website for more information. diff --git a/core/file_area_list.js b/core/file_area_list.js index 9a778468..eeafb4f5 100644 --- a/core/file_area_list.js +++ b/core/file_area_list.js @@ -2,10 +2,8 @@ 'use strict'; // ENiGMA½ -const MenuModule = require('./menu_module.js').MenuModule; -const ViewController = require('./view_controller.js').ViewController; +const { MenuModule, MenuFlags } = require('./menu_module.js'); const ansi = require('./ansi_term.js'); -const theme = require('./theme.js'); const FileEntry = require('./file_entry.js'); const stringFormat = require('./string_format.js'); const FileArea = require('./file_base_area.js'); @@ -77,6 +75,8 @@ exports.getModule = class FileAreaList extends MenuModule { this.fileList = _.get(options, 'extraArgs.fileList'); this.lastFileNextExit = _.get(options, 'extraArgs.lastFileNextExit', true); + this.setMergedFlag(MenuFlags.NoHistory); + if (this.fileList) { // we'll need to adjust position as well! this.fileListPosition = 0; diff --git a/core/file_base_area_select.js b/core/file_base_area_select.js index f936207d..5993f259 100644 --- a/core/file_base_area_select.js +++ b/core/file_base_area_select.js @@ -2,7 +2,7 @@ 'use strict'; // enigma-bbs -const MenuModule = require('./menu_module.js').MenuModule; +const { MenuModule, MenuFlags } = require('./menu_module.js'); const { getSortedAvailableFileAreas } = require('./file_base_area.js'); const StatLog = require('./stat_log.js'); const SysProps = require('./system_property.js'); @@ -24,6 +24,8 @@ exports.getModule = class FileAreaSelectModule extends MenuModule { constructor(options) { super(options); + this.setMergedFlag(MenuFlags.NoHistory); + this.menuMethods = { selectArea: (formData, extraArgs, cb) => { const filterCriteria = { @@ -34,7 +36,7 @@ exports.getModule = class FileAreaSelectModule extends MenuModule { extraArgs: { filterCriteria: filterCriteria, }, - menuFlags: ['popParent', 'mergeFlags'], + menuFlags: [ MenuFlags.NoHistory ], }; return this.gotoMenu( diff --git a/core/file_base_download_manager.js b/core/file_base_download_manager.js index b8564b73..3bebb7d1 100644 --- a/core/file_base_download_manager.js +++ b/core/file_base_download_manager.js @@ -2,10 +2,8 @@ 'use strict'; // ENiGMA½ -const MenuModule = require('./menu_module.js').MenuModule; -const ViewController = require('./view_controller.js').ViewController; +const { MenuModule, MenuFlags } = require('./menu_module.js'); const DownloadQueue = require('./download_queue.js'); -const theme = require('./theme.js'); const ansi = require('./ansi_term.js'); const Errors = require('./enig_error.js').Errors; const FileAreaWeb = require('./file_area_web.js'); @@ -38,6 +36,8 @@ exports.getModule = class FileBaseDownloadQueueManager extends MenuModule { constructor(options) { super(options); + this.setMergedFlag(MenuFlags.NoHistory); + this.dlQueue = new DownloadQueue(this.client); if (_.has(options, 'lastMenuResult.sentFileIds')) { diff --git a/core/file_base_search.js b/core/file_base_search.js index 13e109fb..6b573551 100644 --- a/core/file_base_search.js +++ b/core/file_base_search.js @@ -121,7 +121,6 @@ exports.getModule = class FileBaseSearch extends MenuModule { extraArgs: { filterCriteria: filterCriteria, }, - menuFlags: ['popParent'], }; return this.gotoMenu( diff --git a/core/file_base_user_list_export.js b/core/file_base_user_list_export.js index 57b28ac2..e453c0b3 100644 --- a/core/file_base_user_list_export.js +++ b/core/file_base_user_list_export.js @@ -2,7 +2,7 @@ 'use strict'; // ENiGMA½ -const { MenuModule } = require('./menu_module.js'); +const { MenuModule, MenuFlags } = require('./menu_module.js'); const FileEntry = require('./file_entry.js'); const FileArea = require('./file_base_area.js'); const { renderSubstr } = require('./string_util.js'); @@ -65,6 +65,9 @@ const MciViewIds = { exports.getModule = class FileBaseListExport extends MenuModule { constructor(options) { super(options); + + this.setMergedFlag(MenuFlags.NoHistory); + this.config = Object.assign( {}, _.get(options, 'menuConfig.config'), diff --git a/core/file_base_web_download_manager.js b/core/file_base_web_download_manager.js index 233a247e..fc560b87 100644 --- a/core/file_base_web_download_manager.js +++ b/core/file_base_web_download_manager.js @@ -2,10 +2,8 @@ 'use strict'; // ENiGMA½ -const MenuModule = require('./menu_module.js').MenuModule; -const ViewController = require('./view_controller.js').ViewController; +const { MenuModule, MenuFlags } = require('./menu_module.js'); const DownloadQueue = require('./download_queue.js'); -const theme = require('./theme.js'); const ansi = require('./ansi_term.js'); const Errors = require('./enig_error.js').Errors; const FileAreaWeb = require('./file_area_web.js'); @@ -40,6 +38,8 @@ exports.getModule = class FileBaseWebDownloadQueueManager extends MenuModule { constructor(options) { super(options); + this.setMergedFlag(MenuFlags.NoHistory); + this.dlQueue = new DownloadQueue(this.client); this.menuMethods = { diff --git a/core/menu_module.js b/core/menu_module.js index 335c6e43..afa4c998 100644 --- a/core/menu_module.js +++ b/core/menu_module.js @@ -20,6 +20,23 @@ const assert = require('assert'); const _ = require('lodash'); const iconvDecode = require('iconv-lite').decode; +const MenuFlags = { + // When leaving this menu to load/chain to another, remove this + // menu from history. In other words, the fallback from + // the next menu would *not* be this one, but the previous. + NoHistory: 'noHistory', + + // Generally used in code only: Request that any flags from menu.hjson + // are merged in to the total set of flags vs overriding the default. + MergeFlags: 'mergeFlags', + + // Forward this menu's 'extraArgs' to the next. + ForwardArgs: 'forwardArgs', +}; + +exports.MenuFlags = MenuFlags; + + exports.MenuModule = class MenuModule extends PluginModule { constructor(options) { super(options); @@ -48,6 +65,11 @@ exports.MenuModule = class MenuModule extends PluginModule { }); } + setMergedFlag(flag) { + this.menuConfig.config.menuFlags.push(flag); + this.menuConfig.config.menuFlags = [...new Set([...this.menuConfig.config.menuFlags, MenuFlags.MergeFlags])]; + } + static get InterruptTypes() { return { Never: 'never', diff --git a/core/menu_stack.js b/core/menu_stack.js index 961fdc7e..fcd5f0bf 100644 --- a/core/menu_stack.js +++ b/core/menu_stack.js @@ -5,12 +5,12 @@ const loadMenu = require('./menu_util.js').loadMenu; const { Errors, ErrorReasons } = require('./enig_error.js'); const { getResolvedSpec } = require('./menu_util.js'); +const { MenuFlags } = require('./menu_module.js'); // deps const _ = require('lodash'); const assert = require('assert'); - -// :TODO: Stack is backwards.... top should be most recent! :) +const bunyan = require('bunyan'); module.exports = class MenuStack { constructor(client) { @@ -27,19 +27,11 @@ module.exports = class MenuStack { } peekPrev() { - if (this.stackSize > 1) { - return this.stack[this.stack.length - 2]; - } + return this.stack[this.stack.length - 2]; } top() { - if (this.stackSize > 0) { - return this.stack[this.stack.length - 1]; - } - } - - get stackSize() { - return this.stack.length; + return this.stack[this.stack.length - 1]; } get currentModule() { @@ -56,13 +48,13 @@ module.exports = class MenuStack { return cb( Array.isArray(menuConfig.next) ? Errors.MenuStack( - 'No matching condition for "next"', - ErrorReasons.NoConditionMatch - ) + 'No matching condition for "next"', + ErrorReasons.NoConditionMatch + ) : Errors.MenuStack( - 'Invalid or missing "next" member in menu config', - ErrorReasons.InvalidNextMenu - ) + 'Invalid or missing "next" member in menu config', + ErrorReasons.InvalidNextMenu + ) ); } @@ -81,7 +73,6 @@ module.exports = class MenuStack { prev(cb) { const menuResult = this.top().instance.getMenuResult(); - // :TODO: leave() should really take a cb... this.pop().instance.leave(); // leave & remove current const previousModuleInfo = this.pop(); // get previous @@ -129,7 +120,7 @@ module.exports = class MenuStack { client: self.client, }; - if (currentModuleInfo && currentModuleInfo.menuFlags.includes('forwardArgs')) { + if (currentModuleInfo && currentModuleInfo.menuFlags.includes(MenuFlags.ForwardArgs)) { loadOpts.extraArgs = currentModuleInfo.extraArgs; } else { loadOpts.extraArgs = options.extraArgs || _.get(options, 'formData.value'); @@ -138,7 +129,6 @@ module.exports = class MenuStack { loadMenu(loadOpts, (err, modInst) => { if (err) { - // :TODO: probably should just require a cb... const errCb = cb || self.client.defaultHandlerMissingMod(); errCb(err); } else { @@ -151,22 +141,6 @@ module.exports = class MenuStack { return; } - // - // Handle deprecated 'options' block by merging to config and warning user. - // :TODO: Remove in 0.0.10+ - // - if (modInst.menuConfig.options) { - self.client.log.warn( - { options: modInst.menuConfig.options }, - 'Use of "options" is deprecated. Move relevant members to "config" block! Support will be fully removed in future versions' - ); - Object.assign( - modInst.menuConfig.config || {}, - modInst.menuConfig.options - ); - delete modInst.menuConfig.options; - } - // // If menuFlags were supplied in menu.hjson, they should win over // anything supplied in code. @@ -180,9 +154,9 @@ module.exports = class MenuStack { // in code we can ask to merge in if ( Array.isArray(options.menuFlags) && - options.menuFlags.includes('mergeFlags') + options.menuFlags.includes(MenuFlags.MergeFlags) ) { - menuFlags = _.uniq(menuFlags.concat(options.menuFlags)); + menuFlags = [...new Set(options.menuFlags)]; // make unique } } @@ -193,12 +167,8 @@ module.exports = class MenuStack { currentModuleInfo.instance.leave(); - if (currentModuleInfo.menuFlags.includes('noHistory')) { - this.pop(); - } - - if (menuFlags.includes('popParent')) { - this.pop().instance.leave(); // leave & remove current + if (currentModuleInfo.menuFlags.includes(MenuFlags.NoHistory)) { + this.pop().instance.leave(); // leave & remove current from stack } } @@ -214,17 +184,19 @@ module.exports = class MenuStack { modInst.restoreSavedState(options.savedState); } - const stackEntries = self.stack.map(stackEntry => { - let name = stackEntry.name; - if (stackEntry.instance.menuConfig.config.menuFlags.length > 0) { - name += ` (${stackEntry.instance.menuConfig.config.menuFlags.join( - ', ' - )})`; - } - return name; - }); + if (self.client.log.level() <= bunyan.TRACE) { + const stackEntries = self.stack.map(stackEntry => { + let name = stackEntry.name; + if (stackEntry.instance.menuConfig.config.menuFlags.length > 0) { + name += ` (${stackEntry.instance.menuConfig.config.menuFlags.join( + ', ' + )})`; + } + return name; + }); - self.client.log.trace({ stack: stackEntries }, 'Updated menu stack'); + self.client.log.trace({ stack: stackEntries }, 'Updated menu stack'); + } modInst.enter(); diff --git a/core/message_base_search.js b/core/message_base_search.js index f98b0b89..e102f743 100644 --- a/core/message_base_search.js +++ b/core/message_base_search.js @@ -113,7 +113,6 @@ exports.getModule = class MessageBaseSearch extends MenuModule { const returnNoResults = () => { return this.gotoMenu( this.menuConfig.config.noResultsMenu || 'messageSearchNoResults', - { menuFlags: ['popParent'] }, cb ); }; @@ -160,7 +159,6 @@ exports.getModule = class MessageBaseSearch extends MenuModule { messageList, noUpdateLastReadId: true, }, - menuFlags: ['popParent'], }; return this.gotoMenu( diff --git a/core/msg_area_list.js b/core/msg_area_list.js index 43227a24..0639bc72 100644 --- a/core/msg_area_list.js +++ b/core/msg_area_list.js @@ -2,7 +2,7 @@ 'use strict'; // ENiGMA½ -const { MenuModule } = require('./menu_module.js'); +const { MenuModule, MenuFlags } = require('./menu_module.js'); const messageArea = require('./message_area.js'); const { Errors } = require('./enig_error.js'); const UserProps = require('./user_property.js'); @@ -29,6 +29,9 @@ exports.getModule = class MessageAreaListModule extends MenuModule { constructor(options) { super(options); + // always include noHistory flag + this.setMergedFlag(MenuFlags.NoHistory); + this.initList(); this.menuMethods = { @@ -49,7 +52,7 @@ exports.getModule = class MessageAreaListModule extends MenuModule { extraArgs: { areaTag: area.areaTag, }, - menuFlags: ['popParent', 'noHistory'], + menuFlags: [ MenuFlags.NoHistory ], }; return this.gotoMenu( diff --git a/core/msg_conf_list.js b/core/msg_conf_list.js index 74439be6..0769ed47 100644 --- a/core/msg_conf_list.js +++ b/core/msg_conf_list.js @@ -2,7 +2,7 @@ 'use strict'; // ENiGMA½ -const { MenuModule } = require('./menu_module.js'); +const { MenuModule, MenuFlags } = require('./menu_module.js'); const messageArea = require('./message_area.js'); const { Errors } = require('./enig_error.js'); @@ -26,6 +26,9 @@ exports.getModule = class MessageConfListModule extends MenuModule { constructor(options) { super(options); + // always include noHistory flag + this.setMergedFlag(MenuFlags.NoHistory); + this.initList(); this.menuMethods = { @@ -49,7 +52,7 @@ exports.getModule = class MessageConfListModule extends MenuModule { extraArgs: { confTag: conf.confTag, }, - menuFlags: ['popParent', 'noHistory'], + menuFlags: [ MenuFlags.NoHistory ], }; return this.gotoMenu( diff --git a/core/my_messages.js b/core/my_messages.js index 50bba7ae..0f1fe620 100644 --- a/core/my_messages.js +++ b/core/my_messages.js @@ -2,7 +2,7 @@ 'use strict'; // ENiGMA½ -const MenuModule = require('./menu_module.js').MenuModule; +const { MenuModule, MenuFlags } = require('./menu_module'); const Message = require('./message.js'); const UserProps = require('./user_property.js'); const { filterMessageListByReadACS } = require('./message_area.js'); @@ -16,6 +16,7 @@ exports.moduleInfo = { exports.getModule = class MyMessagesModule extends MenuModule { constructor(options) { super(options); + this.setMergedFlag(MenuFlags.NoHistory); } initSequence() { @@ -49,7 +50,6 @@ exports.getModule = class MyMessagesModule extends MenuModule { if (!this.messageList || 0 === this.messageList.length) { return this.gotoMenu( this.menuConfig.config.noResultsMenu || 'messageSearchNoResults', - { menuFlags: ['popParent'] } ); } @@ -58,7 +58,6 @@ exports.getModule = class MyMessagesModule extends MenuModule { messageList: this.messageList, noUpdateLastReadId: true, }, - menuFlags: ['popParent'], }; return this.gotoMenu( diff --git a/core/show_art.js b/core/show_art.js index 2359081c..9cbe303f 100644 --- a/core/show_art.js +++ b/core/show_art.js @@ -4,7 +4,6 @@ // ENiGMA½ const MenuModule = require('./menu_module.js').MenuModule; const Errors = require('../core/enig_error.js').Errors; -const ANSI = require('./ansi_term.js'); const Config = require('./config.js').get; const { getMessageAreaByTag } = require('./message_area.js'); @@ -21,6 +20,7 @@ exports.moduleInfo = { exports.getModule = class ShowArtModule extends MenuModule { constructor(options) { super(options); + this.config = Object.assign({}, _.get(options, 'menuConfig.config'), { extraArgs: options.extraArgs, }); diff --git a/core/upload.js b/core/upload.js index ec73a233..c055acb2 100644 --- a/core/upload.js +++ b/core/upload.js @@ -2,7 +2,7 @@ 'use strict'; // enigma-bbs -const MenuModule = require('./menu_module.js').MenuModule; +const { MenuModule, MenuFlags } = require('./menu_module'); const stringFormat = require('./string_format.js'); const getSortedAvailableFileAreas = require('./file_base_area.js').getSortedAvailableFileAreas; @@ -76,6 +76,8 @@ exports.getModule = class UploadModule extends MenuModule { constructor(options) { super(options); + this.setMergedFlag(MenuFlags.NoHistory); + this.interrupt = MenuModule.InterruptTypes.Never; if (_.has(options, 'lastMenuResult.recvFilePaths')) { diff --git a/docs/_docs/admin/oputil.md b/docs/_docs/admin/oputil.md index b2ee2892..f1463cc9 100644 --- a/docs/_docs/admin/oputil.md +++ b/docs/_docs/admin/oputil.md @@ -323,7 +323,7 @@ qwk-export arguments: | Action | Description | Examples | |-----------|-------------------|---------------------------------------| -| `import-areas` | Imports areas using a FidoNet style *.NA or AREAS.BBS formatted file. Optionally maps areas to FTN networks. | `./oputil.js config import-areas /some/path/l33tnet.na` | +| `import-areas` | Imports areas using a FidoNet style *.NA or AREAS.BBS formatted file. Optionally maps areas to FTN networks. | `./oputil.js mb import-areas /some/path/l33tnet.na` | | `areafix` | Utility for sending AreaFix mails without logging into the system | | | `qwk-dump` | Dump a QWK packet to stdout | `./oputil.js mb qwk-dump /path/to/XIBALBA.QWK` | | `qwk-export` | Export messages to a QWK packet | `./oputil.js mb qwk-export /path/to/XIBALBA.QWK` | diff --git a/docs/_docs/configuration/menu-hjson.md b/docs/_docs/configuration/menu-hjson.md index 9c358109..a376a8bd 100644 --- a/docs/_docs/configuration/menu-hjson.md +++ b/docs/_docs/configuration/menu-hjson.md @@ -59,13 +59,15 @@ The `config` block for a menu entry can contain common members as well as a per- | `menuFlags` | An array of menu flag(s) controlling menu behavior. See **Menu Flags** below. #### Menu Flags -The `menuFlags` field of a `config` block can change default behavior of a particular menu. +The `menuFlags` field of a `config` block can change default behavior of a particular menu: | Flag | Description | |------|-------------| -| `noHistory` | Prevents the menu from remaining in the menu stack / history. When this flag is set, when the **next** menu falls back, this menu will be skipped and the previous menu again displayed instead. Example: menuA -> menuB(noHistory) -> menuC: Exiting menuC returns the user to menuA. | -| `popParent` | When *this* menu is exited, fall back beyond the parent as well. Often used in combination with `noHistory`. | -| `forwardArgs` | If set, when the next menu is entered, forward any `extraArgs` arguments to *this* menu on to it. | +| `noHistory` | When leaving the current menu to load/chain to another, remove this menu from history. In other words, the fallback from the next menu would *not* be this one, but the previous. | +| `mergeFlags` | Generally used in code only: Request that any flags from `menu.hjson` | +| `forwardArgs` | Forward this menu's `extraArgs` to the next. | + +> 💡 In JavaScript code, `MenuFlags` from `menu_module.js` contains constants for these flags. ## Forms diff --git a/docs/_docs/modding/show-art.md b/docs/_docs/modding/show-art.md index 69be395d..8fd84b10 100644 --- a/docs/_docs/modding/show-art.md +++ b/docs/_docs/modding/show-art.md @@ -58,7 +58,6 @@ showFileBaseAreaArt: { method: fileBaseArea cls: true pause: true - menuFlags: [ "popParent", "noHistory" ] } } ``` diff --git a/misc/menu_templates/file_base.in.hjson b/misc/menu_templates/file_base.in.hjson index 8b102c68..a4fc1b11 100644 --- a/misc/menu_templates/file_base.in.hjson +++ b/misc/menu_templates/file_base.in.hjson @@ -388,7 +388,7 @@ art: FEMPTYQ config: { pause: true - menuFlags: [ "noHistory", "popParent" ] + menuFlags: [ "noHistory" ] } } @@ -779,7 +779,7 @@ art: FBNORES config: { pause: true - menuFlags: [ "noHistory", "popParent" ] + menuFlags: [ "noHistory" ] } } @@ -807,7 +807,7 @@ art: FBNORES config: { pause: true - menuFlags: [ "noHistory", "popParent" ] + menuFlags: [ "noHistory" ] } } @@ -852,7 +852,7 @@ art: ULNOAREA config: { pause: true - menuFlags: [ "noHistory", "popParent" ] + menuFlags: [ "noHistory" ] } } diff --git a/misc/menu_templates/message_base.in.hjson b/misc/menu_templates/message_base.in.hjson index d20d3077..a5fcebc8 100644 --- a/misc/menu_templates/message_base.in.hjson +++ b/misc/menu_templates/message_base.in.hjson @@ -776,7 +776,7 @@ key: confTag pause: true cls: true - menuFlags: [ "popParent", "noHistory" ] + menuFlags: [ "noHistory" ] } } @@ -794,7 +794,7 @@ key: areaTag pause: true cls: true - menuFlags: [ "popParent", "noHistory" ] + menuFlags: [ "noHistory" ] } } }