search

akavel / up

Ultimate Plumber is a tool for writing Linux pipes with instant live preview
Apache License 2.0
8.39k stars 129 forks source link

Modify the deprecated func #42

Open ujun opened 5 years ago

ujun commented 5 years ago

The document of tcell (https://godoc.org/github.com/gdamore/tcell#Screen) says as following.

SetCell is an older API and will be removed. Please use SetContent instead

This PR alters SetCell into SetContent .

ujun commented 5 years ago

As the doc also says that the fourth argument will usually be nil, I set nil to that for the present.

akavel commented 5 years ago

I believe the way the new API in tcell is better, is that it allows for supporting combining runes. Would you consider adding full support for those as part of this PR, as part of a comprehensive, "full steam ahead" migration to the new API? I must admit I'd really love, were we to do it, if we did it right...

Without that, I'm currently somewhat hesitant towards just changing the call sites from one signature to the other; especially given the fact, that with the old signature, we kinda implicitly signal, that we are in fact obsolete and "incorrect" (by blindly ignoring the need for special handling of the combining characters). This way, we're hinting between the lines at the fact, that this should indeed be changed to SetContent; but then, ideally by having it done in the "correct" way, with support and recognition of Unicode combining characters. I don't really have any idea what such support would take; I just never tried to do it; it may well be really simple to do (there's the whole unicode package, which may well have some ready-made functions which might make this functionality trivial, or at least easy enough). If you were to try exploring this direction, it would really help me make my mind on this change. Would you consider trying? Or have maybe you already tried, and could share some findings from this exploration, maybe showing how it's too hard to do?

With all that said, please note I am not yet well aware of all the reasons you had to find this change worth doing and submitting. Could you let me know a bit more of the background behind what helped you decide on that? I'd be really interested in learning more, if you don't mind telling me! This could help me understand how to try and approach this change and this conversation appropriately for both of us. TIA! And by all means, please do note I'm really happy and grateful for your attempt at helping me with the project, and really appreciate it, thanks a lot! :smiley:

ujun commented 5 years ago

@akavel

Thanks for your reply. First of all, I'm sorry I didn't make the purpose of this PR clear enough.

I too think that this PR shouldn't be merged without full support . As I'm so new about handling Unicode, I hoped somebody to help me through this PR.

Though I thought I should tackle this on my own, wanna do anything better for this repos and carelessly made this PR.

You can close this PR for now. Sorry for disturbing you.

Best Regards,

akavel commented 5 years ago

@ujun -san,

I'm very sorry, I would be really grateful if you could try to not take my words as critique or offensive, I absolutely didn't intend this! I'm really super happy, and really appreciate it, that you found the project so interesting, that you took time and effort to want to help me with it, and submit a PR! It's just that I needed a lot of clarification on various things; that's why I tried to ask, in such a way I was able to. Communication on the Internet is, in my opinion, unfortunately often difficult, and sometimes much more words need to be spilled until a common understanding is reached, than if speaking eye to eye. Please try to be patient with me, I know I may not understand Japanese culture and ways well enough, I have big respect for them and for you, I would never want to offend you, and always try all my best to be polite; I would be really grateful if you could try to always be patient for me, and any faux pas I may have done, especially as a Westerner, when talking with you, and I'm afraid I may do again, alas, not even knowing that I do them! I never in my life had contact with Japanese culture, and way of speaking and communication. So there is nobody, other than you, that can tell me if I did something offending or impolite in your eyes, and teach me your ways. But you are also always free to leave this conversation and relationship at your discretion, if you ever find you are tired and don't have enough patience for me; I already have huge respect for you, and it will stay so, so I will always respect your decision.

I want to also add, that I would be really very happy to try and help you through this PR, if you would be interested in my guidance and further discussion. That's something I especially respect and appreciate in people who decide to submit a PR for projects I tend to. That you said so in your reply, was very important for me and made me really happy for you, and makes me believe there is a good perspective that we could hopefully cooperate to try and lead this project to improve and become better, as a carefully tended garden. I strongly believe, that cooperation on a PR is always a very interesting learning experience for both sides, both on the professional side, and on personal side of things.

I would be very sorry to close this PR, if you still have time and will of heart to try and improve it. It made me really sorry that you wrote about it as careless; I don't see your PR as such; on the contrary, I strongly believe that the fact that you made it is in itself an expression that you do care. It's just that I didn't understand initially enough to know more about your plans, ideas, context, background, etc, etc. Without those, a PR with a short message is unfortunately usually too difficult for me to understand and merge without some further questions and discussion.

For your convenience, based on what I managed to find for now on the Internet, for developing this PR, I thought that maybe those links could be of some initial help:

If you would have any questions, or doubts, or problems, I would be honored, and really glad, and really respect you if you decided to ask me about them, ideally in this conversation; I will do my best to answer them the best I can, or try to lead you in some potentially promising direction. I would also really appreciate if you could share your plans regarding this PR and how you think you might improve it, if it's OK for you that we would then discuss them.

Also, by the way, if we manage to successfully lead this PR to completion, I would be really interested if you liked to help the project to be useful with Japanese characters. I have no idea if it works OK now!

Thank you very much, and sorry for any ways that I may have written in a way not polite enough, or may do so again and again in future, even though I really wish this would never happen.

Best Regards,

akavel commented 5 years ago

PS. On the other hand, based on https://github.com/leighmcculloch/go-strrev/blob/master/strrev.go#L79 (via https://stackoverflow.com/a/46903574/98528), it might be possible, that just the unicode.IsMark function might be enough for up? It would be good to try and understand this area somewhat better.

ujun commented 5 years ago

@akavel

Thank you for your reply. And thanks for much information. I really think this project is wonderful.

You are not making me angry or annoyed. Rather, I am very pleased to meet such a kind person on the Internet for the first time.

I had no confidence and I was about to give up. Thank you for encouraging me. It may take quite some time, but I will try it.

Please give me some time.

ujun commented 5 years ago

@akavel

I'm sorry I took time.

It looks like it works well with the fixes I have committed additionally considering combining characters. Also, within my observation range, it seems that Japanese is working well.

The following modules that you have already added to go.mod helped a lot: https://github.com/mattn/go-runwidth

Also, unicode.IsMark () could be useful returning if rune is a combining character. https://golang.org/pkg/unicode/#IsMark

There have been a lot of fixes, but would you please review it?

Regards,