Crius mac oldenly patch 1#3186
Crius mac oldenly patch 1#3186CriusMacOldenly wants to merge 12 commits intoMudlet:developmentfrom CriusMacOldenly:CriusMacOldenly-patch-1
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. |
SlySven
left a comment
There was a problem hiding this comment.
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.