Full support edit CRLF(win) files in editor - #5151
Conversation
github-actions
Bot
added
needs triage
ace0ef3 to
65b4540
Compare
|
Fix some editor behaviour in mixed EOL-type files and code clang-format CI tests |
|
I'd like to make such large commits a bit smaller:
|
zyv
added
area: mcedit
|
I apologize, all the code was generated by the AI agent, and I reviewed it as best I could. I even just tested the expected behavior in the editor and made corrective prompts (though some navigation issues are still possible, and I'll be using the modified editor for work now; I might find more bugs). I couldn't use the editor at all on WIN files until I decided to do something about it. |
Add three text buffer helpers that will be used to treat a Windows
("\r\n") line break as a single unit:
- edit_buffer_is_crlf() checks whether the buffer contains a "\r\n"
line break starting at the given position.
- edit_buffer_detect_line_breaks() detects the line break type of the
buffer content: LB_WIN if all line breaks are "\r\n", LB_UNIX if all
are "\n", LB_MAC if all are "\r" and LB_ASIS for a mixture.
- edit_buffer_trailing_ws_start() returns the start offset of the
trailing whitespace (spaces and tabs) run of a line; for a CRLF line
the line content ends at the "\r" of its line break, not at the "\n".
Signed-off-by: Kuzin Andrey <kuzinandrey@yandex.ru>
Collect all line break insertion into a single edit_insert_line_break() helper and use it instead of direct edit_insert (edit, '\n') calls in edit_double_newline(), check_and_wrap_line() and the CK_Enter/CK_Return key handling. This is a pure refactoring without any behavior change; the helper will be extended with the line break type inheritance in a follow-up commit. Signed-off-by: Kuzin Andrey <kuzinandrey@yandex.ru>
…s and detect line break type
Show the line break type of the file being edited as a fifth character
on the status line: U - all "\n", W - all "\r\n", M - all "\r",
- - a mixture of line breaks or no line breaks. The type is detected
by edit_buffer_detect_line_breaks() and cached in the buffer
(edit_buffer_refresh_line_breaks()/edit_buffer_get_line_breaks()).
Hide the "\r" of a "\r\n" line break in files with pure Windows line
breaks. In any other file (a mixture of line breaks, Unix or Mac) the
"\r" is shown as "^M", so CRLF lines stay visible, e.g. in a patch
containing diffs of files saved with both "\n" and "\r\n". A
standalone "\r" (Mac) is rendered as "^M" instead of as a tab.
A "\r\n" line break is a single unit in every file: the End key and
Delete/Backspace stop at / remove the whole pair, line counting and
cursor columns ignore the "\r" part. A line break inserted with Enter
inherits the type of the current line ("\r\n" or "\n"); for the last
line of a file the previous line's type is used. The "\r" of the pair
is not an editable character: pressing Enter at the end of a CRLF line
does not duplicate the "\r" and the cursor stays before it.
File content is kept raw in the buffer: mixed line breaks are
preserved as-is on save (the default LB_ASIS mode), the Save As dialog
still offers conversion to a single line break type. Paragraph
formatting skips CRLF lines as reformatting would mangle them, and the
search end-of-line symbol is always "\n".
Fix edit_write_stream() appending an extra "\n" to files that already
end with a line break in conversion modes.
Add unit tests for line break detection, conversion, inheritance and
atomic deletion of CRLF lines in files with mixed line breaks
(tests/src/editor/edit_line_breaks.c).
Signed-off-by: Kuzin Andrey <kuzinandrey@yandex.ru>
65b4540 to
55dce2c
Compare
|
i'm certain that we already had a contribution attempting that a while ago. i can't find it, which is unfortunate, because i didn't like it, and i'd like to see it contrasted with what we have here. the way you factored out the first commit without usage and tests isn't very elegant. a personal note on AI: i don't care if you use it, but if you use it as an excuse for anything, you are so out. |
|
I don't want to waste a lot of time shuffling lines between commits, covering them with tests, and other nonsense just to get the PR accepted. Either the community is happy with the commits and they accept them (they can change them to suit their own vision), or I just use them in my private branch - they solved a big problem for me very well. I don't want to be responsible for AI slop, so I'm warning you about it right away; I've verified as best I can that it at least works. I apologize for using AI, because people accepting PR will spend their personal time analyzing what the machine generated, and even finding errors and flaws in it. Many open-source projects have switched to ignoring AI slop and are unwilling to accept it. |
I linked it at the top of the ticket. |
Just a friendly tip: consider that this is a public forum. If you are an engineer, it might be unwise to declare proper engineering practices "nonsense". Someone considering employing you in the future might read it. Anyhow, that's up to you, but I'm afraid it'll then stay in your private branch. Also, there is no "development community" of mc. I wonder where you guys get all these strange milk river and unicorn ideas from? You must be too long out of school to believe in Santa Claus. I'm seriously curious. Who do you think develops mc and why they do it, and how do they make a living? In reality, currently there are 1.5 overworked maintainers looking at the project in their non-existent spare time, trying to keep it alive - answering issues, commenting on PRs, doing some development work, that's it. So either contributors put the time to make the contribution quality match the minimum acceptable safe floor for us to merge (and the standards are very low), or the patches will just hang around until someone will rework them - which might happen later or never.
I personally have no problems with AI-assisted contributions whatsoever, as long as the contributors are the ones who put their time supervising their agents and fixing the code, instead of offloading it to the maintainers along the lines of "look what I have generated, I didn't check it and I don't understand it, and I won't change it according to your requests, but it works for me and I'll keep using it, so take it or leave it". |
|
I apologize if this seemed disrespectful to your work, but it's not. I understand perfectly well how various corporations make billions off the unpaid work of two open source project enthusiasts, so I don't want to waste your time. But my experience also involves spending weeks of work making some contribution to the project, polishing every commit to a high standard, only to have it sit abandoned for years. For example, libevent/libevent#1753 (but that's the problem with project abandonment). Even at work, I often struggle with MR due to QA tests, indentation and syntax corrections, and various bureaucratic hurdles, and it's often demotivating. I want to bring proposed changes to the highest possible quality, but moving lines between commits and writing tests for things that can be done without them is enough for my personal use. My work involves developing a Linux distribution, and I can say with certainty that 95% of projects don't have any unit tests because open source enthusiasts simply don't have the time. |
i don't see it. you linked the issue. the PR (presumably still on trac) was about a decade younger. iirc, it was just converting at read and write. |
The patches from 2011 and 2013 are attached at the top of the linked issue. The PR from 2014 is also linked there: MidnightCommander/mc-old#49 . |
|
I used a slightly different approach and implemented configurable rendering of control characters in mcedit. In the normal mode, they are displayed using caret notation, such as ^M, ^L, and ^? etc... When the option is disabled, each control character is rendered as a single blank cell, while the file contents and the bytes stored in the editor buffer remain unchanged. Screen-column calculations follow the selected mode: a visible control character occupies two columns, while a hidden one occupies one. When the mode is toggled, the layout cache is invalidated and the cursor position is recalculated. As a result, the cursor remains attached to the same byte, and all open editor windows are updated immediately. This solution required relatively few changes because it affects only the presentation of the data, not the editor’s internal data model. Loading, saving, searching, insertion, deletion, and undo continue to operate on the original bytes without special handling. The option uses the existing settings, menu, and keymap infrastructure, while a small shared width function keeps column calculations consistent across the relevant code paths. https://github.com/blue-panels/mc6/pull/223/changes If there is interest in this particular approach, it should be relatively easy to adapt it for upstream. |
|
Overall, the CRLF-handling approach used in the patch author’s branch is workable. I tried a similar approach some time ago, but eventually abandoned it, and the corresponding patches never made it upstream. |
|
Oh, now that I see that: I wish you could adopt a different name for your fork other than "mc6" before this spreads. It produces an impression of being a new incompatible version of one and the same project (and the default implication is that it is maintained by the same team), and "Midnight Commander with Plugins" is also a very weak differentiator for the users. We are already regularly getting reports from the Windows fork, and here the total confusion is simply guaranteed. I don't want to have to interact with the users of your project, and probably you don't want to get reports for our issues either. Besides, there is an even worse binary naming conflict as with the Windows fork. At least we don't support Windows, and we are not going to. But in the Unix world, there are already two It can be really anything you want, but please not yet another Midnight Commander. If you want to be able to take the name of |
Regarding the binary name conflict, I have already opened an issue in our tracker to rename the main binary to mc6 blue-panels/mc6#219 . This should avoid a conflict with mc, but it is not a trivial change: the current binary name is referenced in many places, and all those cross-references need to be identified and updated consistently. |
I shared the link to the commit not to promote or discuss the fork, but to demonstrate the approach I used in my implementation. If anyone finds it useful, it can be adapted for upstream with almost no changes. As for the fork itself, I doubt it will attract any significant influx of users: dual-pane file managers are, after all, a fairly niche type of software. Therefore, the risk of widespread confusion seems limited to me, although I understand the concern about the name. |
|
@ilia-maslakov I'm asking because when I proposed a commit to display the current Git branch in the file panel (which is very convenient for me without call |
A while ago, Ilya did reach out to me privately, to which I replied that due to the history of our previous interactions, I'd prefer not to work together in the future. Since then, he has submitted a few PRs. I think that some of these changes were fine, but I have other interests and priorities, and some of them I disagree with on technical grounds. I think that it's absolutely great that he subsequently created his own fork, where he can do whatever he thinks is right, and however he thinks is right, without us having to interact, and I sincerely wish him all the success with it. This is a right decision that will hopefully work well for everybody. The only thing I take a serious problem with is him calling it "Midnight Commander with Plugins" and using the name "mc6", now that I was made aware of it. I hope that we don't need to escalate this, and he can simply rename it to whatever he thinks is acceptable, now that I asked once, and repeated my request for the second time. He already called the organization "Blue Panes", so why not "Blue Panes" and "bp"? Or anything really, just not "Midnight Commander" and "mc*". There are dozens of clones and forks of double pane managers, and all of them somehow managed to find a distinctive name. Squatting a name used by a different project and refusing to change it is pretty much a definition of hostile forking. I do believe that he really doesn't mean to do anyone any harm and genuinely thinks it's okay. This illustrates, however, why I don't want to work together. We have different ideas, and clashing is not good for anyone. The Internet is a big place, I hope that everybody can find their own spot and co-exist peacefully. |
You don't have to apologize for anything, as I didn't take any offense. I was just trying to tell you that you are free to do whatever you think is right for you, and so am I, and actually anyone out there. If you don't want to put any more effort into this PR for whatever reason - you don't even have to justify it, just leave it at that, that's absolutely fine. |
|
@zyv Yuri, thanks for your reply. This only confirms my point that maintainers always have their own opinion and can accept or reject contributors' changes. So, to avoid wasting each other's time, I simply adopted the principle of "either accept it or not." For a contributor, what they created for their own use is always sufficient, and there's no need for any special engineering principles (minimum working functionality is sufficient). Therefore, if a maintainer rejects these changes, that's fine; no one owes anyone anything. |
I understand your concern, i have already opened an issue to rename the mc binary to mc6 to avoid the naming conflict. |
In the case of displaying the current Git branch, the problem, in my opinion, is not merely the subjective usefulness of the feature. The proposed implementation is too narrowly scoped and adds a synchronous external command invocation to mc's core code through popen("git branch --show-current 2>&1"). Even if the feature can be disabled through an option, this call introduces additional failure modes. The git process may not terminate promptly, causing mc to block while waiting for the result. The implementation must also correctly handle a missing git executable, process startup and pipe-reading errors, exit codes, unexpected output, and slow or unavailable filesystems. It is this synchronous external command invocation that introduces additional instability into mc. |
don't carry, i already create issue to rename mc to mc6. |
I did not merely try to contribute changes to the main project - I was part of the Midnight Commander team for many years, and even then it was neither quick nor easy. Unfortunately, a personal tragedy caused me to withdraw almost completely from the development process for several years. Getting changes upstream presents an objective challenge. New features require prior agreement, discussion, reconsideration of details, further discussion, and resolution of any remaining rough edges. In my own project, I was able to close around 80 PRs in six months. At the same time, trying to move two small changes upstream - one of which was a one-line fix for a crash - took two or three months and made very little progress. I understand the need for such caution: Midnight Commander runs on an enormous number of combinations of hardware platforms, operating systems, and distributions, so changes must be verified very carefully. This is not a complaint about the team or the process. From a practical perspective, however, I am not sure that I have several years to coordinate and upstream the entire set of changes I would like to implement. |
I don't think you understand my request. I'm not asking you to rename your binaries to
The names like "Midnight Commander with Plugins" and Will you do this, or are you refusing to do it? |
I will add to the Readme next: "This is an independent, unofficial fork of GNU Midnight Commander. It is not affiliated with, maintained by, or endorsed by the GNU Midnight Commander team or the Free Software Foundation. Please report issues related to this fork to this project, not upstream." Could you please clarify on whose behalf you are making this request, who owns the rights to the names “Midnight Commander” and “mc”, and provide the applicable trademark policy, registration, or other legal basis for the request? The fork already clearly identifies itself in the mc6 -V output: Midnight Commander with Plugins, a fork of GNU Midnight Commander (blue-panels/mc6), changes since 2026-02. I am currently considering renaming it to "Blue Panels - an extended fork of GNU Midnight Commander". But, GNU GPLv3 does not require modified versions or forks to be renamed. Section 5 requires modified versions to be clearly identified and the modifications to be documented, which I have done. If the use of “Midnight Commander” or “mc” is prohibited on trademark or another legal basis, could you please identify the rights holder and provide the applicable trademark policy or other legal basis for that restriction? proof:
Rus: Юр, я не очень понимаю мотивов, но если есть какие то реальные причины и резоны которые запрещают использовать словосочетание "Midnight commander", я таких не знаю т.к. ранее я проверял нет ли нарушений с точки зрения FSF, то напиши о них, сейчас твои слова что кто то перепутает звуча как придирки. Но и тратить время за приперательства желания нет. т.е если есть законные основания то конечно переименую. |
|
Are there similar suggestions to rename mc² to something else for this fork as well? https://www.geek.co.il/~mooffie/mc-lua/docs/html/ ? |
I have no problems with you calling it "Blue Panels" and clarifying whatever you want however you want wherever you want, as long as your project is not called "Midnight Commander XXX" and "mc[0-9]+". You are entitled to refer to Midnight Commander however you like, as long as you don't identify your project as "Midnight Commander something" / mcX.
I'm making this request on my own behalf as one of the maintainers of the existing "Midnight Commander" project identified as
In my jurisdiction (Germany), the (trade)mark law stipulates the same protections for the marks (project names) which have demonstrably acquired public recognition (Verkehrsgeltung) as the explicitly registered trade marks (see § 4 1. MarkenG): https://www.gesetze-im-internet.de/markeng/__4.html It is correct that the copyright law universally doesn't regulate the usage of project names, and it doesn't have anything to do with the project license either.
Законные основания, если речь идет о моей юрисдикции, базируются на гражданском кодексе, и конкретно на законе о названиях, который предусматривает одинаковую защиту как для зарегистрированных названий, так и названий (проектов, продуктов и т.п.), которые находятся в определенной области в общем употреблении. Надеюсь, что это объяснение тебя удовлетворит, ты переименуешь свой форк, и мы сможем на этом прекратить наше общение. Я не понимаю, почему ты не хочешь придумать свое имя проекта, вместо того, чтобы захватывать уже существующее, и создавать проблему смешения, а также конфликта имен пакетов в дистрибутивах. Сотни форков выбирают себе новые имена и после этого развиваются независимо без каких-либо конфликтов. |
|
Я может быть не в своё дело лезу, просто, как мнение (раз это мой PR, гы %). Находиться можно где угодно, в Германии, Испании, России, Тайланде, США, Канаде, везде законы разные/отличающиеся относительно авторских прав и товарных знаков, кроме того санкции, торговые войны и прочий швах. Но как-то правильно было бы руководствоваться трансграничными правилами касающимся только софта (причём открытого), а в нашем случае это лицензия GPL. А если она, как привёл пункты Илья, не противоречит его действиям, то всё это лишь высказывание субъективных мнений и пожеланий, а противоположная сторона спора может лишь согласиться и принять предложения, либо отказаться без объяснения причин. Либо это должна быть оформленная торговая марка, которая защищается законами большинства стран, если этого нет, то никаких рычагов давления - просто просьбы и убеждения. |
|
I have reviewed your argument regarding German trademark law (Verkehrsgeltung according to § 4 1. MarkenG) and acknowledged the identity of the claimant. While I maintain that my use of the name is fair, constitutes nominative use, and is compliant with GPLv3 (which governs copyright, not trademarks), I have no desire to engage in protracted legal disputes or create package name conflicts in distributions. Therefore, I will proceed with renaming the project to avoid further conflict. Regarding the future naming: "Blue Panels -- an extended fork of GNU Midnight Commander" I expect that this choice resolves all your concerns, as it removes "Midnight Commander" from the product name while maintaining truthful historical attribution. I will also retain the proposed Readme disclaimer: "This is an independent, unofficial fork of GNU Midnight Commander. It is not affiliated with, maintained by, or endorsed by the GNU Midnight Commander team or the Free Software Foundation..." Let's consider this matter closed. |
тут такое дело, что даже если Юрий и не прав, у меня нет ни времени ни желания разбираться с Гитхабом который может грохнуть репозиторий по DMCA по щелчку пальцев, не разбираясь с тем кто прав кто виноват. Устанешь с ними общаться. А мне не хотелось бы тратить остатки своей ценной жизни на препирательства, вместо того чтобы заниматься тем что нравится - писать код. |
Моё уважение... Писать код это возможно пожалуй последнее что осталось в радость... Но и то отнимают, увольняют, заменяют на ИИ... Гитхаб тот вообще продавшись корпорастам перестал быть островом независимости и открытости, а стал набором датасета для обучения кремния, да еще обмазался политотой (но ему по другому нельзя). |
5 participants
Resolves:
Proposed changes
This is opencode AI-driven improvement in editor for support edition of Win (CRLF) files (and with mixed line ends).
Current version of editor didn't support CRLF (win) files (show only ^M at line ends), adding any new line in such file was difficult, because it add all lines in LF (unix) mode. Now it automatically detect current line-end-style in file and can add new lines with right EOL (U - \n, W - \r\n, M - \r).
Before:
After:
Checklist
git commit --amend -smake indent && make check)