ocornut / imgui

Dear ImGui: Bloat-free Graphical User interface for C++ with minimal dependencies
MIT License
60.68k stars 10.25k forks source link

Code column width #3193

Closed richard-vock closed 9 months ago

richard-vock commented 4 years ago

The code does not seem to follow any style guidelines known to mankind.

Considering the "documentation" is also code and even here on github requires a lot of horizontal scrolling to read this is frankly a burden to the user.

As an example here is the documentation of ImGui::Image:

imgui

A quick estimation tells me that ~40% of the code is not readable on the common 80 column restriction. 5 Lines (3 of which are the comment and therefore the "documentation") are not even readable on a 1920x1080 display at full width (font size 11) without horizontal scrolling.

Not to mention that "readable" and "fits onto the screen" are separate issues...

I strongly suggest the authors to open the imgui_demo.cpp here on github, scroll through it and (re)consider using a tool like clang-format.

ocornut commented 4 years ago

I don't disagree with the intent, but there are trade-offs to make. Word-wrapping comments would largely increase code height. Based on screen shape, height is more valuable than width. Not all comments are meant to fully read every-time. There's lots of code, the aim is to allow user to easy "find" a suitable location to look at, and only when you need to get into specific details you want to fully read the comments. If everything was unwrapped it would largely alter that flow. That would also not work with the compact format of imgui.h.

I'm not sure how "80 columns" is common in 2020 - e.g. GitHub which you are point out shows 140 columns. That's a genuine question I'm asking here! Visual Studio with a side-bar gives me 180+ characters, with two simultaneous code window opens and without a side-bar I get 110.

I don't mind settling on a consistent limit (e.g. <140 columns) but 80 seems largely too small.

richard-vock commented 4 years ago

It is indeed correct that 80 is quite small and tbf I myself use more columns, but I wanted to understand the Image function and here's what happened:

  1. I was on mobile and it was impossible to read. Also horizontal scrolling is notoriously bad in some mobile browsers.
  2. I used my desktop PC and it was still pretty hard to read the comment when being forced to horizontal scroll for every single line of text.
  3. I checked out the project and opened it in VIM on my full size screen (resolution I stated above): Same problem.
  4. I figured out how to enable line-wrap (VIM problems).
  5. Reading the example absolutely did not help to understand my flipped UV issue (but that is a different topic) so I read the actual internal code.

Note that points 2-5 were steps I exclusively had to do for imgui - and I read a lot of code written by others. Also consider that some of us have to work in e.g. tmux sessions over ssh, not a huge IDE window (especially during a global pandemic-induced lockdown ;) ).

So some limit would be nice. Common linters and formatters usually ship a variety of popular presets to draw inspiration from - some if not the majority with a limit above 80 chars (however there are reasons for 80: Google Pros and Cons.)

Another approach you implicitly mentioned already: If github uses 140 columns that would seem like a reasonable choice - after all an "example.cpp" is something I actually read on github before I consider using - and cloning - a library. And if your functions (at least in the example code) exceeds the screen height with a 140 column limit it might hint at an overly complex function - at least in theory ;).

Edit: One thing I forgot: A restriction on the code width might also benefit you when looking at diffs of commits/branches. Unless you are used to that good old unix diff style where differences are printed linewise with "<<<" and ">>>" markers...

ocornut commented 4 years ago

Hello,

I have modified imgui_demo.cpp to mostly fits within 120 columns now (added +320 lines of code to imgui_demo.cpp). Some lines or functions made less sense to wrap so I didn't apply this everywhere but rather where I felt it made most sense.

-Omar

PazerOP commented 4 years ago

What is the downside to using word wrap in your editor? That way it will fit your screen no matter what horizontal size it is, and it won't affect existing codebases that already fit your window.

ocornut commented 9 months ago

Closing this old issue now.

To address some of your feedback in more details: In May 2020 and onward i have made an effort to reduce use of long lines/comments in imgui_demo.cpp, in particular targeted to multi-line comments or strings literals. I had another pass today and wrapped some more (mostly in sections that have been added since).

but I wanted to understand the Image function and here's what happened: [...] Reading the example absolutely did not help to understand my flipped UV issue (but that is a different topic) so I read the actual internal code.

I have now added extra commentary around ImGui::Image() (both .h and .cpp)

// Widgets: Images
// - Read about ImTextureID here: https://github.com/ocornut/imgui/wiki/Image-Loading-and-Displaying-Examples
// - 'uv0' and 'uv1' are texture coordinates. Read about them from the same link above.
IMGUI_API void Image(ImTextureID user_texture_id, const ImVec2& image_size, const ImVec2& uv0 = ImVec2(0, 0), const ImVec2& uv1 = ImVec2(1, 1), const ImVec4& tint_col = ImVec4(1, 1, 1, 1), const ImVec4& border_col = ImVec4(0, 0, 0, 0));

First line was added a while ago (but after your post), second line added today.

I have amended the "Textures Coordinates" section of WIKI page to add another example showing flipping (simpler than subsequent examples): https://github.com/ocornut/imgui/wiki/Image-Loading-and-Displaying-Examples#about-texture-coordinates