If the previous build had a long error message, I have to scroll to the right. The next build is still scrolled to the right and I can't see all the normal length messages until I manually scroll back to the left. This resets the size of the treeview when a build is started. You can view, comment on, or merge this pull request online at:
https://github.com/geany/geany/pull/3816
-- Commit Summary --
* [build] Reset compiler treeview width at start of build
-- File Changes --
M src/build.c (2)
-- Patch Links --
https://github.com/geany/geany/pull/3816.patch https://github.com/geany/geany/pull/3816.diff
@ntrel pushed 1 commit.
b1643f4e4e9e2f918ea8b5a5c56d0e5d896755b2 Same for Find in Files, Find Usage
Or maybe [wrap](https://docs.gtk.org/gtk3/property.CellRendererText.wrap-mode.html) could be turned on.
@elextr That would probably be a nice option to support, but in the case of very long lines the option could be annoying and cause a lot of extra scrolling if you are looking for something near the start of a line. So given that there are reasons not to use wrap, I think we should reset the width.
I'm not so sure there is a "reason not to use wrap", you have made a speculation, but have you tried it? In Vscode wrapped error messages (G++) are still the crap the compiler gives, but at least there is no need to scroll side to side to try to figure out what its whinging about, its possible to read it all.
Thats not to say this PR isn't an improvement, I just was looking to see if there might be more improvement available.
@elextr BTW this applies to search as well. Often when doing Find in Files I'm not interested in any matches on long lines (e.g. text files), I'm only interested in matches in code which are almost always short lines. Wrapping would just add more vertical noise.
Often when doing Find in Files I'm not interested in any matches on long lines (e.g. text files), I'm only interested in matches in code which are almost always short lines. Wrapping would just add more vertical noise (in this case).
See your use-case and raise you a different use-case.
But then if searching documentation where modern markups tend to use single line paragraphs edited with wrap on it would be very useful as most of the matching line would be off screen and the horizontal scrolling would be horrible :-)
If you don't want to add wrap just don't do it, I am only asking, but don't try to justify it with easily refutable use-cases.
@elextr Of course we should support wrapping, all I was saying was that wrapping is not always an improvement.
all I was saying was that wrapping is not always an improvement.
Ok, true, which suggests having a menu item to turn it on and off quickly and easily, not a preference.
Ok, true, which suggests having a menu item to turn it on and off quickly and easily, not a preference.
Yes.
Or maybe [wrap](https://docs.gtk.org/gtk3/property.CellRendererText.wrap-mode.html) could be turned on.
I've tried that - see #3834.
Merged #3816 into master.
@ntrel, where does anybody say they have tested or reviewed this?
@elextr It was open for a week, you didn't object to the change after seeing it, it's an obvious improvement. There's no reason why either treeview should be wider than necessary.
It was open for a week
Availability of our contributors is limited, a week is nowhere near time that guarantees people have time to notice, and review it. Most only have weekends, but if they are busy one weekend they have missed your PR.
you didn't object to the change after seeing it
I suggested an alternative approach and we are looking at that on another PR, that is clearly not approval of this change.
it's an obvious improvement.
Because we are looking at an alternative I have not even looked at the implementation of this, so "obvious" is purely your own judgement. It may be right in this case, but it may not be in another. Having someone else review your changes is part of a cooperative project, not merging before they have a chance.
Each of your rebuttals is valid on its own. Taken together though, my points justify the merge IMO.
It may be right in this case, but it may not be in another.
There are no other cases. I thought we agreed that wrapping should not always be on, and this does not conflict with wrapping.
Having someone else review your changes is part of a cooperative project, not merging before they have a chance.
Reverting is not the end of the world.
@ntrel You seem to have misunderstood the point of my objection. The problem was the process, merging a change before others had the chance to assess it, not that there was a technical problem with the PR. Since as I noted I had not had time to see its contents I could not be commenting on the content.
Taken together though, my points justify the merge IMO.
This was not a crash problem or a sudden regression, it was an inconvenience that Geany users had been living with for a long time, there was no need to hurry a merge without giving others time to assess it. There is no release planned for tomorrow, so there is no rush to merge to enable distribution of the change to users. Therefore nothing justifies a quick merge.
There are no other cases. I thought we agreed that wrapping should not always be on, and this does not conflict with wrapping.
Again, I was not taking about the technical content of the PR, sorry if that was not clear. As I said above I was talking generally about the process, "cases" meant applying your sole judgement to the "obviousness" and complexity of the change to justify a quick merge for this and future PRs.
Having had a chance to look at the changes, why not put `reset_msgwin` in `ui_utils` and call it from both `build` and `search`, or is there some technical reason that the order of the calls needs to be different in build? If there is it probably should be commented so someone doesn't reorder them in the future.
Reverting is not the end of the world.
Agree totally, thats a very mature and sensible approach, reverting is not a criticism, it should be thought of in the same way as deleting a line of code if it needs to be changed, but some members of the Geany project seem to have a horror of it :-)
github-comments@lists.geany.org