santaclose / ImGuiColorTextEdit

Colorizing text editor for ImGui
MIT License
69 stars 19 forks source link

Simple warning fixes & General questions about this fork #5

Closed pthom closed 1 year ago

pthom commented 1 year ago

Hi,

This PR proposes some simple warning fixes (correct includes, and usage of ImGui::Text("%s", ...)).


Unrelated to this PR: Some questions about this fork

Also, I am the author of Dear ImGui Bundle. It uses ImGuiColorTextEdit intensively : I have my own fork, in which I patched a few errors that you also corrected in your fork (and you added lots of other functionalities)

I see that this fork is the most maintained fork of the original project. Thanks for publishing your good work! Do you intend to communicate about it as a viable and maintained fork? If so, I would be interested in making your fork the original source that I use in my project. However, I would have several suggestions:

And finally, I have a usage question: I see how to create multiple cursor (via Ctrl-D). However, I do not see a shortcut (maybe via the Esc key) to disable the multi-cursor once it is active. Is this possible via a shortcut?

santaclose commented 1 year ago

Hi, thanks for reaching out

Do you intend to communicate about it as a viable and maintained fork?

Not entirely sure what you mean with this, I'm fixing bugs from time to time and trying to make it better in the little free time I have. The code is not very high quality right now imo and I don't fully understand every single function. I still need to write tests and simplify logic. The editor however seems usable, and you are free to use it.

Could you make it possible to open issues in your fork (at the moment it is only possible to post pull requests)?

Didn't know it was not possible to open issues. I'll take a look.

Could you make your fork more visible? Maybe you should post an issue in the original repo in which the title advertises about your fork.

I find this a bit intrusive.

Could you update the readme and list the modifications you added and their usage (multicursor, find, etc.)

Sounds like a good idea, will try to find time for this.

However, I do not see a shortcut (maybe via the Esc key) to disable the multi-cursor once it is active. Is this possible via a shortcut?

Currently only possible to disable multi-cursor by clicking with the mouse button. I agree that the Escape key should also be handled for this.

pthom commented 1 year ago

Thanks for your answers

pthom commented 1 year ago

This is just FYI:

I updated Dear ImGui Bundle so that it is based on my fork which is then based on your fork: Details at https://github.com/pthom/imgui_bundle/blob/main/external/ImGuiColorTextEdit/Readme_Forks.md

santaclose commented 1 year ago

Thanks for letting me know 🙂