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

Fixed issue on URL resolver message #552

Merged
merged 4 commits into from
Aug 31, 2024
Merged

Conversation

stack-fault
Copy link
Contributor

Moved user_to uppercase to a different function
This fixes issue #551

Moved user_to uppercase to a different function
@@ -245,8 +245,7 @@ exports.getModule = class MrcModule extends ServerModule {
connectedSockets.forEach(client => {
if (
message.to_user == '' ||
// Fix PrivMSG delivery on case mismatch
message.to_user.toUpperCase() == client.username.toUpperCase() ||
message.to_user == client.username.toUpperCase() ||
Copy link
Owner

Choose a reason for hiding this comment

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

These would technically be better using localeCompare with sensitivity 'accent'.

// Make sure to_user is always uppercase
try {
let to_user_uc = to_user.toUpperCase();
to_user = to_user_uc;
Copy link
Owner

Choose a reason for hiding this comment

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

This could just be to_user = to_user.toUpperCase(), no temp var. Or without the try:

to_user = (to_user || '').toUpperCase()

- Added msg delay on connect to space out commands
- Fixed linewrap issue
- Added MRC Trust verbs
- Added PM local echo
- Fixed to_user and from_user uppercase
@stack-fault
Copy link
Contributor Author

Ok, I did a couple things in here to fix things I noticed while testing.
I will open another issue for something that is buried down elsewhere for which you may have an idea.

@stack-fault
Copy link
Contributor Author

@NuSkooler let me know if you have additional questions before merging the PR


const padding = ' |00' + ' '.repeat(padAmount);
chatLogView.addText(pipeToAnsi(msg + padding));
chatLogView.addText(pipeToAnsi(msg));
Copy link
Owner

@NuSkooler NuSkooler Aug 29, 2024

Choose a reason for hiding this comment

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

I'm not sure this will work correctly, I'll have to do some testing. This MR just got a bit bigger

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have the latest client and it worked fine on my end. All the padding appeared to be from an older way of drawing the screen which appeared to have changed since it was first implemented (assumption here). On my end, it was working pretty well without it and fixed an issue with text wrapping on certain cases. Let me know if there is anything I missed.

Fixed previously added passthru (TRUST)
Fixed trailing PIPE code sent to server unnecessarily
@stack-fault
Copy link
Contributor Author

@NuSkooler
Pushed some additional small fixes to the branch since it's not merged yet.

// room names are displayed with a # but referred to without. confusing.
room = room.replace(/^#/, '');
this.state.room = room;
this.sendServerMessage(`NEWROOM:${this.state.room}:${room}`);

await this.msgDelay(100);
Copy link
Owner

Choose a reason for hiding this comment

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

nice!

Copy link
Owner

@NuSkooler NuSkooler left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks for your work on this!

@NuSkooler NuSkooler merged commit 30d2ac1 into NuSkooler:master Aug 31, 2024
2 checks passed
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