-
-
Notifications
You must be signed in to change notification settings - Fork 247
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
Crius mac oldenly patch 1 #3186
Crius mac oldenly patch 1 #3186
Conversation
Hey there! Thanks for helping Mudlet improve. 🌟 Test versionsYou can directly test the changes here:
No need to install anything - just unzip and run. |
Could you please fill in the title and description for this PR? |
There is some information in the original issue here: #612 (comment)
|
This PR seems somewhat strange. Also, your last commit (the only relevant here) seems to add a new file in the root folder, and not modify the existing file in src/ directory. Thanks for looking into this! |
We covered this in Discord today - @CriusMacOldenly is super new to Git and they wanted to include changes from the other PR in this one, so we'll go easy :) |
Did you also discuss the new/updated file issue? |
Yeah... enough pain from git for @CriusMacOldenly for today I think, I'll sort this out 😅 |
Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
Also - your Git (or your code editor) is incorrectly set up as it is putting in carriage-returns as well as line-feeds (Window style End-Of-Lines) at the end of each line instead of the standard which we used which is just line-feeds (*nix style End-Of-Lines). Which ballses things up for doing |
I am just putting together a PR against your proposal that should clean things up here... wait a few minutes please... |
I made certain a checkmark was placed in: |
Okay can you go to https://github.com/CriusMacOldenly/Mudlet/pull/1 ... and follow the instruction I have offered... |
Convert: change Windows EOL to Unix EOL in previously modified file
Success! Now the "Files changed" tab at the top of the page shows the actual changes you want to make... |
Indeed. I can now see 6 lines I commented out, but forgot to delete. :( Should have only been +92 -198 in the end. I'd edit the file again and remove them, but that'll probably bother you a lot. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've taken a quick look at this - and thought "this is not going to be a simple one to review" - I am sorry but apart from several good cups of coffee I think it is going to take a while for me to get back into gear over the code in this bit and I fear I will not be able to give a meaningful reply without some serious playing with it over the next few days. The good news is that I do have some time off for all of the next fortnight...
Before pushing another change out to your github repository I wonder if there is a way to see whether there are CR-LF or just LF on the ends of the lines in the files that get changed. I note that it was only the |
You need to separate declaring y1 and assigning it because normalizeSelection() assigns mPA, which is then assigned to y1. |
Humm, 🤔 so every file you touch gets converted to Windows style end-of-lines and committed as such. If you go to the place in your file-system where the Mudlet source code is and type in a command line:
does it report anything? |
D:\Mudlet>git config core.autocrlf |
Any other changes before I commit your requested y1 declare and assign change? |
Please hold... you are in a queue and the next available operator will be with you as soon as possible... 🙂 |
Oh, I already committed the change you requested, and deleted the commented out lines like I wanted. |
Your call is important to us, please hold and the next available operator will be with you as soon as they can - alternatively you might like to check out our on-line presence on the Interweb thingy... I'm discussing the EOL issue on Discord - if you want to go to the |
This patch introduces an issue of a quick flicker where the split pane appears and disappears when starting to scroll up with the mouse alone. Vadi linked a video in https://streamable.com/jycjg . |
Brief overview of PR changes/additions
Fix for Scrollbar does not move with mousewheel/pg scrolling #612
Motivation for adding to Mudlet
I was scrolling with the mouse wheel, and was greatly bothered when I clicked on the scroll bar and it reset the scroll position as if I had never used the mouse wheel to scroll at all.
Other info (issues closed, discussion etc)
This change has my previous changes because this seems to be a patch, with no ability to do my own new fork/pull request, and it was much easier to fix this bug than figure out what github wanted.