Scrollback patch request

jackpeters667 commented 3 years ago

Sweet work buddy. Would you be able to provide a scrollback patch here or on the suckless site? So everyone can share your awesomeness?

ashish-yadav11 commented 3 years ago

I will but it can take a while.

apprehensions commented 3 years ago

waits very patiently for it ( amazing work btw! )

ashish-yadav11 commented 2 years ago

I haven't had time yet to extract the patch. But thanks to koetemagie, you can find one here. I haven't looked at the patch yet. If required I will refine it and update here.

BeyondMagic commented 2 years ago

If required I will refine it and update here.

It would be better, since you made it, you know what should and shouldn't be on the patch.

apprehensions commented 2 years ago

https://gist.github.com/wael444/bc44d877d0f6fe96411c49d265dbe82d is this good?

ashish-yadav11 commented 2 years ago

There are some unnecessary parts but otherwise I don't see any problem.

ashish-yadav11 commented 2 years ago

The STRESCARG thing isn't required. I have separately provided it in "title parsing fix" titled patch on suckless' site.

apprehensions commented 2 years ago

The STRESCARG thing isn't required. I have separately provided it in "title parsing fix" titled patch on suckless' site.

oh. well it was included in the fused patch that was by BeyondMagic well could you modify the diff and remove it? or is it easy to just replace all lines that look like STRESCARGEST to strescseq.args?

ashish-yadav11 commented 2 years ago

yeah just remove STRESCARG and every change in strhandle function.

apprehensions commented 2 years ago

STRESCARGREST to strescseq.args and STRESCARGJUST to what? it is only used in par = (narg = strescseq.narg) ? atoi(STRESCARGJUST(0)) : 0;

ashish-yadav11 commented 2 years ago

remove every change in strhandle function. Keep it vanilla and remove the definition of STRESC... at the top.

apprehensions commented 2 years ago

could you check on the new revision of the gist? i also assume that the patch name should be added to the scrollback patch section with the name st-scrollback-column-reflow-20220410-ef05519.diff

BeyondMagic commented 2 years ago

The scrollback is maintained by another person, and I'm pretty sure how the scroll works in this patch is too different to add there (In my opinion, obviously).

apprehensions commented 2 years ago

honestly, i just formatted the patch the main authors or whoever is related can choose what the patch name and description should be if i were to submit the package id have to wait on @ashish-yadav11 's call on if the patch is good to be submitted. then make its name seen in my previous comment.

apprehensions commented 2 years ago

for some reason, now my patch sets a broken title.

my only fix was to patch in title parsing fix.

ashish-yadav11 commented 2 years ago

remove the changes in strparse function.

apprehensions commented 2 years ago

remove the changes in strparse function.

well that fixed it. i didn't know there were still strparse changes, thanks!

also, is this still good to be submitted yet? or is there still some problems?

ashish-yadav11 commented 2 years ago

You can submit if you want to. Do add a link to this repo in case of bugs or for support.

apprehensions commented 2 years ago

You can submit if you want to. Do add a link to this repo in case of bugs or for support.

alright.. i will just do a pretty basic description for that matter. i will also update your patch title parsing fix since it doesnt apply cleanly

apprehensions commented 2 years ago

a new scroll back patch was released. st-scrollback-0.8.5, i did not base my patches over this new one.

https://gist.github.com/wael444/fcfe9fd27c7f0c75f45aa9275d0abca0 contains both the patch that is placed ontop of scrollback >=0.8.5 and another patch which fuses both scrollback 0.8.5 and column reflow.

are they good?

btw. the patch will be named column_reflow just keep that in mind as i have no other name for it.

ashish-yadav11 commented 2 years ago

If after patching you get the same code, then it must be fine.

apprehensions commented 2 years ago

sorry for the constant comments, i will be putting this under the scrollback patch index.md as this the original patch requires to patch it on top of scrollback 0.8.5 as it makes sense for it to be added there, unless BeyondMagic's comment is correct and this should be placed under a different patch index as of how different it is.

i also believe by adding this to the scrollback patch page it'll be much easier for users to find this patch.

apprehensions commented 2 years ago

it breaks st-scrollback-mouse-20220127-2c5edf2.diff, it no longer scrolls.

edit: turns out the patch is actually malformed and is missing some things...

apprehensions commented 2 years ago

https://st.suckless.org/patches/scrollback/

well, this should be closed now i guess

ashish-yadav11 / st

Scrollback patch request #2