screen library: ncurses vs ncursesw #3628
Comments
I'm very confused about this ticket. What is wrong with the current situation? It seems to me that option (4) makes total sense.
As far as I understand, ncursesw is the focus of current development (whereas ncurses is no longer actively developed), and it is source-compatible with ncurses, but behaves better in response to user locale settings.
Therefore, if both ncurses and ncursesw are available, and ncurses is selected as the screen library, it makes total sense to silently prefer ncursesw instead. Using ncurses if ncursesw is available doesn't bring any advantages, which would justify offering a choice. Those, who want to force ncurses for some reason can patch it themselves, but most likely they don't know what they are doing.
What am I missing? |
Currently Debian compile failed in combination of --with-screen=ncurses and libncursesw5-dev.
If no, why we have a screen ncursesw flag? |
Reflect about that issue again.
In point of mc maintenance its more practicable to have only -with-screen=ncurses
So, agreement for that? |
I don't get it; in the end, you want to change from (4) to (2), is that correct? |
Replying to zaytsev:
Jep |
|
|
|
|
|
|
Remove undocumented ncursesw flag. ncurses and ncursesw selection by --with-screen=ncurses only. Signed-off-by: Andreas Mohr <and@gmx.li> Signed-off-by: Andrew Borodin <aborodin@vmail.ru>
|
@aborodin If you don't want to open a PR, but just link a branch, unfortunately no notifications are sent and the status of the branch is not clear. The CI is failing... My suggestion for the workflow is this:
What do you think? |
|
On Fri, 28 Feb 2025 22:58:23 -0800 "Yury V. Zaytsev" ***@***.***> wrote:
@aborodin If you don't want to open a PR, but just link a branch,
I dislike PRs and don't have any plans to use them for my branches.
`git pull/merge/push` are enough for me.
The CI is failing...
I've fixed that.
My suggestion for the workflow is this:
1. Without PR
1. Assign issue to yourself
2. Link a branch
3. When ready for review, leave comment with reviewer's @mention
Ok.
|
The only reason I'm advocating PRs is that otherwise it's a pain to leave comments on the lines in the patch while reviewing. In a PR, you can highlight any line and add a comment or suggestion. However, I can now leave comments on commits that belong to branches. I hope that you will be notified by email, and that you will be able to reply by email. If you want to use branches, this might work... The only problem is the lack of overview. With PR, all comments remain visible, with branch/commit comments there is no page where you can see them all. But I'm very happy with the progress so far...
Just note that you will need to rebase and force-push your branch to remove fixups before merging. In the past, this was not necessary, and you could rebase locally and merge to the master without force-pushing the branch. I changed this to prevent PR merges without rebasing using the web interface. If you have a problem merging, please let me know and I will look into it. I haven't tried the manual way yet.
... for now, I have added a fixup directly to your branch. Otherwise, approved 👍
I forgot about updating the milestone. I need to add this list to the (Trac) wiki. |
mind elaborating why? forbidding direct pushes would have the benefit of not needing commit notifications to stay on top activity, plus the obvious benefit of having all commits reviewed, if only by the committer himself. if you have a problem with the GH interface specifically, how about gerrit? i'm all for bridging to gerrithub.io, which i'd use with my client scripts. |
Do you have a good example of how Gerrit feels these days? When I looked at it a decade ago, it was actually quite okay, but unfortunately focused on single commits (one commit = one review, with subsequent iterations). This might be fine for some (large?) projects, but I felt that it mostly introduced too much overhead compared to small branches with a few topical commits being reviewed as a batch, so I settled on Upsource for my company (after they somewhat fixed the handling of force-pushes). I'm now very happy with how GitHub handles force-pushes and added commits with respect to line comments, and I really enjoy pull requests, which I didn't before. So is there still a good argument for Gerrit? |
|
well, actually, the focus on atomic commits is the primary strength of gerrit, and conversely why i find GH reviews "suboptimal", even if it got better over the years. reviewing the batch as a whole can still be done on GH, as gerrithub is a bidirectional bridge (supposedly; i haven't actually used it properly yet). on the client side not much changes if you rebase/squash everything into shape anyway, and with my scripts it's almost as if one was direct-pushing, except that the actual integration is delayed. note that integration can be done with a single merge for an entire series when the repo is configured that way, so the resulting history should look as before if you want it that way. |
|
On Sat, 01 Mar 2025 05:02:36 -0800 "Yury V. Zaytsev" ***@***.***> wrote:
zyv left a comment (MidnightCommander/mc#3628)
... for now, I have added a fixup directly to your branch. Otherwise, approved 👍
Approved or not? Is there any way to make this info as special label/state/whatever?
Is there any way mo implement states "on review", "on hold", "on rework" for issue?
|
"Otherwise approved" means that I approve if you approve my fixup.
The only way to do this with issues would be to add some labels, and make sure you manually add and remove them. However, doing exactly that will poorly replicate the functionality of pull requests, just with a different, less convenient interface. With pull requests:
But if you don't want to use PRs, why do you need labels? Why can't we just agree that comments in issues summarize state, like "ready for review" or "approved"? |
|
On Sun, 02 Mar 2025 00:37:53 -0800 "Yury V. Zaytsev" ***@***.***> wrote:
> Approved or not? Is there any way to make this info as special label/state/whatever?
"Otherwise approved" means that I approve if you approve my fixup.
Ok.
> Is there any way mo implement states "on review", "on hold", "on rework" for issue?
The only way to do this with issues would be to add some labels,
Fine. Let's do that.
and make sure you manually add and remove them.
No problem.
why do you need labels? Why can't we just agree that comments in issues summarize state, like "ready for review" or "approved"?
It should be a special mark not a plain words among a lot of words of a lot of comments. It should be found instantly not after reading of comments.
|
Oi wei. So which labels do you want to work with (please give me a list)? |
|
On Sun, 02 Mar 2025 02:29:15 -0800 "Yury V. Zaytsev" ***@***.***> wrote:
Oi wei. So which labels do you want to work with (please give me a list)?
At least
state:on review
state:approved
Optional
state:on rework
state:on hold
|
zyv
added
the
state: approved
Okay, let's start with the first two. They should work now. |
Remove undocumented ncursesw flag. ncurses and ncursesw selection by --with-screen=ncurses only. Signed-off-by: Andreas Mohr <and@gmx.li> Signed-off-by: Andrew Borodin <aborodin@vmail.ru>
* 3628_ncursesw: Extend version print by glib/screen version Restructure ncurses term define Restructure ncurses defines. Restructure with-screen ncurses m4 logic Ticket #3628: ncurses vs ncursesw.
mc-worker
removed
the
state: approved
Important
This issue was migrated from Trac:
andCurrently mc support following screen library (--with-screen=) selection:
This leads to imprecision for ncurses,
because autoconf logic of ncurses detection deals (partly) with ncursesw too.
1) Exact match/use of ncurses or ncursesw
to give full control about ncurses selection.
2) Single ncurses switch
with ncursesw lib preference if both libs found.
3) Single ncurses switch
with ncurses lib preference if both libs found.
4) Ncurses and ncursesw switch
where ncurses switch also prefer ncursesw lib if both libs found.
(current state)
Before I attach patches, which way is preferred?
(I prefer exact match selection)
Note
Original attachments:
andonApr 10, 2016 at 19:35 UTCandonApr 10, 2016 at 19:36 UTCandonApr 10, 2016 at 19:36 UTCandonApr 10, 2016 at 19:36 UTCandonApr 10, 2016 at 19:36 UTCThe text was updated successfully, but these errors were encountered: