https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_3954

This block of code here seems to be replicating the functionality of `bbs.menu()` except with the addition of a bunch of new @-codes. Why wouldn't we just add new @-codes to sbbs instead? Having one special display file that supports a different subset @-codes from all the others, doesn't seem like a good system design.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_3955

You don't need to use regular expressions here. These are simple text search/replace operations. Regular expressions are very heavy-weight and should only be used when necessary.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_3956

You're re-opening, reading and parsing the configuration file on every menu display. Instead, I would do this operation upon script startup one time only.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_3957

There's already a load library (load/cfglib.js) that is intended for the purpose of simplifying the reading of ctrl/????.ini files. It doesn't yet support reading/parsing main.ini, but that is where I would add this logic (and then use that library for this functionality).

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_3958

Either reuse the `f` variable (and don't declare as const) or use a longer/better variable name.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_3959

Don't hard-code the screen clearing. Put the screen clear code in the display file instead.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_3960

Instead:
`while(bbs.online && !js.terminated) {`

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_3962

I don't understand why you're using a custom node status when "changing defaults" is already a standard node action.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_3961

Remove commented-out code.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_3963

You don't need to call `backslash()` here, just use a forward-slash path separator.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_3965

Why hard-code the prompt rather than use what's already configured in the text.dat file (`UserDefaultsWhich` string)?

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_3966

Don't hard-code the supported archive types here.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_3964

Instead:
`if(user.security.exemptions & UFLAG_T)`

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_3967

You're doubly assigning to `user.settings` here:

replace with:
`user.settings ^= USER_BATCHFLAG;`

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_3969

Check for console.aborted too. If a user hits Ctrl-C abort the string input, there's reason to validate and re-write their configured netmail address.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_3968

Use `bbs.transhcan()` instead of `system.trashcan()` here and then the `text/badtrash.msg` file would be displayed here automatically.

No need to introduce a new hard-coded string.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_3970

Use K_TRIM mode flag to `getstr()` instead.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_3971

Remove commented-out code.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_3972

Use K_TRIM getstr() mode flag instead, here too.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_3973

Use `console.crlf()` instead.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_3975

Why not just `return;` ?

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_3974

Same: use K_TRIM instead.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_3976

You don't need the == value parts here:
`on_or_off(user.settings & USER_EXPERT)` would work fine.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_3978

The prompt line appears to be in this file, yet you also have a prompt hard-coded in the script.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_3977

You already have the screen clear code here, so you don't need the `console.clear()` call in the script.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_3979

Most users don't normally see this option.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_3981

We could do that, or it shows how a user can add their own config options if they want to if they don't exist. I have no objection to you adding them to sbbs.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_3982

But would be moot if you add them to sbbs right?

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_3983

Noted. I will look at that.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_3984

I'm not comfortable messing with those files. If you would like to add that then I'd be happy to use it.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_3985

Noted.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_3986

Noted. Thank you.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_3987

Noted, thank you.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_3988

Yup, I will do that. I only spotted that after I committed it.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_3989

True. I will change it to use the existing node status.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_3990

I stole that from someone else who shall remain nameless. I'll fix it.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_3991

Why use one over the other? Both seem to use used about in equal amounts , chat_sec.sh, for example, uses what I am using here.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_3992

Because this version is customized for my BBS look and feel, once it is working I'll make it look like the default sbbs. That'll probably be the last item that I do.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_3993

Isn't that how it's done now? I thought I traced that routine back to

filedat.c:const char* supported_archive_formats[] = { "zip", "7z", "tgz", NULL };


I'd be happy to use something else. Open to suggestions.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_3994

Noted. Thank you.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_3995

OK. When searching, I only saw 2 lines with bbs.trashcan compares to much more with system.trashcan which is why I used that. I will switch upon your suggestion.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_3996

Good info. Will do.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_3997

ok. Will do. I got this snipped from newuser_signup.js I think.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_3998

You learn something new every day...

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_3999

Good question.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4000

I thought I tried that without having any luck for some reason. I'll give it another try. I guess that goes for the rest of those?

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4001

Yup, again, due to my bbs customization. That'll be fixed before final release.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4002

Which option? The default to quiet mode? I can't remember why I put that in, that's something from a while ago when I was doing something in baja. I'll have to revisit. Can be removed easily enough.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4003

"own config options" that are very similar to, but not identical to, the standard @-codes used in all other Synchronet display files. That seems very confusing.

If you were going to introduce an application-specific set of text-replacement-macros, I'd suggest using another escape scheme (not @-codes) to avoid confusion with the @-codes already supported and in use in sbbs.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4004

Yup.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4005

The stock chat_sec.js does not do any explicit exemption or restriction flag checking. If script knows the specific security element it wants to check for (i.e. it's not a configured string or other sysop-input), then it's faster (less code executed) to just check it directly rather than going through the ARS parsing engine.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4006

The prompt was in the display file too though (?).

And rather than hard-coding, just make it a configurable string with the default being the text.dat string and then in your local modopts.ini you over-ride it with your customized BBS look string.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4007

The user defaults code (in `useredit.cpp`) uses the merged result of the built-in libarchive-supported archive types (defined in `supported_archive_formats` in C) *and* the compressible file types configured in SCFG->File Options (readable from `file.ini`).

I would suggest we use cfglib.js to get the sysop-configured archive types from the file.ini file (after support is added) and then combine them with the hard-coded list.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4008

Right, all of them.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4009

Only users with the 'Q' exemption can see the "Default to Quiet Mode" option. Similarly, only users with the 'V' exemption can see the "AutoLogon via IP Address" option (which you don't have represented here).

In general, using the template scheme won't easily support options that appear/disappear depending on the current user and sysop configuration.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4011

I can certainly leave this out. I was thinking that with the template, if a user doesn't have a permission then I can just exclude printing that line with a condition. I may be over thinking it bit, but yeah, the dynamic options may be a little more difficult.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4012

Yeah because, the default prompt has "Which or Quit?" my shell doesn't offer that so I put it in the template. Like I said, I'll make things look like "standard" synchronet once I'm done getting things working on my board.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4013

Sounds good. I'll wait on you for the cfglib.js bits then. I've never used it before so I'll have to take a look and see if I can figure it out. Thanks.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4014

Got it, thanks for the explanation.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4015

text/badtrash.msg doesn't exist on my system. It's not in git either.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4016

Answer, because it says return is not a function.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4017

The sysop can create that file (correct name would be `bademail.msg` not `badtrash.msg`) if they want text displayed when a string is disallowed.
https://wiki.synchro.net/config:filter_files

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4018

Just `return;` not `return();`?

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4019

And yeah, return is not a function: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/return
But should still be able to use it.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4020

So what I'm saying is: make the default use the text.dat string with a modopts.ini option to over-ride that and then over-ride that string on your system. Then it'll look how you want on your system, but would be "standard" (at least by default) everywhere else.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4021

I this is a better idea than even adding a bunch of new @-codes to sbbs: define a JS object in your script with some properties for the Yes/No or On/Off etc. strings you want, e.g.
```
const disp_strings = { autoterm: autoterm, charset: console.charset, autohang: `on_or_off(user.settings & USER_AUTOHANG), etc. };
```
and then pass that object to bbs.menu() or bbs.printfile() and in the display file use the @JS:propname@ code to display those properties where you want them in the file.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4022

That still doesn't solve the conditional line/option display though. Not sure how you're going to handle that.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4024

'onest gov.

Maybe I should just use exit() ?


---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4025

That had sort of crossed my mind, but wasn't sure if we might want to add properties for the other items. I'm happy to go this route if you like.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4026

So what am I doing wrong here?

const disp_strings = { autohang: on_or_off(user.settings & USER_AUTOHANG), cls: on_or_off(user.settings & USE
R_CLRSCRN) };

I print disp_strings.autohang and disp_strings.cls and they are defined so that's good. I guess I'm now using bbs.menu correctly?

~~~
@CLS@^Ah^AbUser Settings for ^Aw@ALIAS@ #@USERNUM@^An:
^Ah^Ay@HOT@
^Ah^Ay(C) ^Ah^AbClear Screen Between Messages^An^Ab: ^Ac@JS:cls@
^Ah^Ay(Z) ^Ah^AbDefault Download Protocol ^An^Ab: ^Ac ^Ah^AbAuto hangup^An^Ab: ^Ac@JS:autohang@
^Ah^Ay(W) ^Ah^AbChange Password or Signature
^Ah^Ah^AbEnter option or [^AwQ^Ab]uit:@EOF@
~~~

I pared the file down just for testing and it doesn't display anything at all.

bbs.menu(system.text_dir + "menu/default_settings.asc",null,disp_strings);

and I get no errors.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4027

The `null` argument there is an object, so it would be taken as the scope argument. Remove that.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4028

According to jsobj

bbs.menu(base_filename [,mode=P_NONE] [,object scope])

So I figure the 2nd parameter is the mode. Have I been reading these wrong? I actually tried putting P_NONE but no change.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4029

If I were writing this script, I would first write it so that it emulates the current hard-coded behavior using separate (sometimes conditional) print statements, using the text.dat strings, and make the custom "template" or display file an optional feature (which I'd probably add afterwards).

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4030

`return;` only works from within a function. It appears this code is not within a function, so that's why the error. `exit()` would be fine in that case.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4031

I do plan to add much or all of the @-codes you suggested still though, the script just won't have as much control over the display (e.g. "On" or "Off" versus "Yes" or "No") without modifying the text.dat strings at run-time.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4032

bbs.menu(system.text_dir + "menu/default_settings.asc",disp_strings);

This still doesn't work.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4033

Double-check what you put in the .asc file against the property names in `disp_strings`.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4053

For the sake of getting this pushed out, I'll remove the template option for now, instead pointing to users to use replace_text or their own text.dat file. I may look at adding templating later once I've experimented a bit.

Have now accounted for the various I,D,W,M,and E options and I think I have added the security around them properly.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4054

Removed all eotl customization and will use system defaults.

and yes, I'll fork a copy next time :smile:

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4055

Not sure if I've done it right, but I went with taking the array and adding from file.ini compressor: if the lower case version of extension isn't present, otherwise it would show zip and ZIP which I'm not sure we'd want, it looks odd.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4085

Remove debug stuff.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4086

That doesn't look right. if 'i' is 0, then you don't want to use array index i-1. I think the `-1` here just needs to be removed.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4087

You shouldn't need to manually clear the `console.aborted` flag, it's cleared automatically.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4088

Ditto (no need to do this clearing).

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4089

So, you'd get both `text/bademail.msg` displayed to the user (if it exists) and this new string. Probably best to just go back to using `system.trashcan()` here, but I also don't like adding a hard-coded string that can't be easily localized (for other languages) or customized by the sysop. Consider making this overridable via modopts.ini

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4090

You don't need to pass the string argument ('') here to `getstr()` since you're not editing an existing string.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4091

Ditto here, don't need to pass '' to `getstr()` in this case.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4092

This looks like a new log message not present in the current built-in default settings editor. I'm okay with it, but I didn't think you were adding new features/capabilities with this script.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4093

You don't need to assign twice to user.settings here (and elsewhere), just: `user.settings ^= USER_EXPERT;`
Is all you need.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4100

in useredit.cpp line 1057-1058

bputs(text[PasswordChanged]);
logline(LOG_NOTICE,nulstr,"changed password");

Is that not what logline is doing?

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4102

Yes, my bad, I see it now.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4104

My bad. I thought I got them all.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4105

Fixed

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4106

Ditto fixed.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4107

It looks like useredit.cpp doesn't check for entries in email.can so maybe I should remove that until someone requests it? As you said in an earlier note, I should be trying to replicate what is already available and not add new features.

If the feature is added, then we should add a new line to text.dat to accommodate a new "Invalid email" string. For now, I'm removing and we can add back later.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4108

It doesn't appear this way.

case 'E':
if(console.noyes(bbs.text(UseExternalEditorQ))) {
writeln("aborted? " + console.aborted);

When I press CTRL-C at this point, it returns console.aborted: true.


---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4111

Yes, that's correct/expected. And then at the next prompt for input, it'll be automatically reverted back to false. You're not expected to need to manually reset this flag.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4112

If I remove the -1 then if I select 2 for 7zip is gets set to tgz.
If I select 3 for tgz then I get test. If I select 1 for zip I get 7z.

If I set it back to -1 then 1 = zip, 2=7z, 3=tgz and 4=test, CTRL-C leaves it unchanged. The only issue is if you press enter, it doesn't return the right value. I'll look into that.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4113

Bah. Done.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4115

Oh, I see. I think the problem is that you're passing `i+1` to the calls to `uselect()` in the loop above. You don't need to do that.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4136

I think you mean to use `argv` (to get the number of the user to edit) here, not use `user.number`.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4137

Excessive parenthesis, instead:
`if(system.settings&SYS_PWEDIT && !(useron.security.restrictions&UFLAG_G))`

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4138

Excessive parenthesis.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4139

Above, `defaultext` is initialized as a string (''), yet here you're setting it to a number. I suggest just leaving the initial value undefined.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4140

I don't think this will do what you want. Instead:
`if (user.number == useron.number)`

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4141

Again, this `user == useron` comparison isn't going to do what you want. Instead, use `user.number == useron.number`.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4142

I think you meant to put the closing parenthesis before the `=== 0` or something. Really, I think you meant to say:
`if(bbs.text(MouseTerminalQ).length && console.yesno(bbs.text(MouseTerminalQ)))`

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4143

Comparing the return value of `bbs.text()` to `0` here doesn't make sense. I think just removing the `== 0` should work or add `.length` as I suggested above. Whatever method you use, be consistent.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4144

That should be `console.autoterm |= USER_PETSCII;`

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4145

The use of whitespace is inconsistent in this script. I suggest choosing a style (for this script) and sticking to it. Based on other examples in this file, this line should have been written:
`useron.settings |= USER_RIP;`

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4146

Ditto. You're either putting spaces around operators, or you aren't. But sometimes do and sometimes not (in a brand new script) isn't good.

When you see inconsistencies in other source/scripts, it's usually because they've been around a long time and styles have changes or different contributors have contributed with different preferences. With a brand new script, that shouldn't be the case.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4147

Excessive parenthesis:
`if(!useron.compare_ars(file_cfg.protocol[code].ars) || file_cfg.protocol[code].dlcmd.length === 0)`

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4148

I don't understand the creation/use of this additional shells[] array. main_cfg.shell is already an array. And don't appear to ever use the contents of the array after, just its length.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4155

Fixed.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4156

I think the former based on the C++ code. Not that I'm a C++ coder...

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4157

I'm used to perl which pretty much wants parenthesis around everything.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/merge_requests/331#note_4173

I still can't get TYPE=ANSI. If you select No for auto detect and then select no for all the options you'll get

[T] Terminal Mode : US-ASCII / DUMB (mono) (mouse)

Now, go through and select Yes for auto detect, no for iCE colors, yes for mouse reporting, yes for IBM support

[T] Terminal Mode : Auto CP437 / DUMB (color) (mouse)

I'm still getting DUMB as the terminal type.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net