Hi All,
One area of Geany has been annoying me for some time.
At the moment project settings mix session related settings and truely project settings. The session settings are really user specific whilst the project settings are project related. This means that if the project file is in VCS it keeps updating as the session settings change. You can turn session settings off, but then you lose that functionality.
The proposal is to separate these, with the session settings stored in the user config directory under a projects subdir and the project settings stored wherever the user wants. The options ~/projects or in the tree would be available as they are now.
To prevent the config dir getting too cluttered the session files can be kept only as long as the project remains in the recent projects list. (maybe needs a separate length setting, currently uses file_prefs.mru_length)
I will have some time next week, so this thread is to get ideas or objections sorted before then.
My first suggestion is that the [indentation], [long line marker], [project] and [build-menu] sections of the current project file are project specific whilst [files] and [VTE] are session specific.
Cheers Lex
On 11-10-31 04:50 PM, Lex Trotman wrote:
Hi All,
One area of Geany has been annoying me for some time.
At the moment project settings mix session related settings and truely project settings. The session settings are really user specific whilst the project settings are project related. This means that if the project file is in VCS it keeps updating as the session settings change. You can turn session settings off, but then you lose that functionality.
The proposal is to separate these, with the session settings stored in the user config directory under a projects subdir and the project settings stored wherever the user wants. The options ~/projects or in the tree would be available as they are now.
To prevent the config dir getting too cluttered the session files can be kept only as long as the project remains in the recent projects list. (maybe needs a separate length setting, currently uses file_prefs.mru_length)
I will have some time next week, so this thread is to get ideas or objections sorted before then.
I like the idea of having workspaces/profiles, so that on startup (with option to not ask again) and then later through a menu, you can choose one of your profiles to load the settings like, indentation, long-line marker, colour scheme, etc.
IMO, projects should only store things like build commands, paths to project files, project meta-data, etc.
Sessions should be user-transparent things like window geometry, number of instances opened, last project/files, etc. and maybe should be workspace/profile-specific.
I think this is how most other IDEs I've used handle the situation if I'm not mistaken.
Cheers, Matthew Brush
On 1 November 2011 11:49, Matthew Brush mbrush@codebrainz.ca wrote:
On 11-10-31 04:50 PM, Lex Trotman wrote:
Hi All,
One area of Geany has been annoying me for some time.
At the moment project settings mix session related settings and truely project settings. The session settings are really user specific whilst the project settings are project related. This means that if the project file is in VCS it keeps updating as the session settings change. You can turn session settings off, but then you lose that functionality.
The proposal is to separate these, with the session settings stored in the user config directory under a projects subdir and the project settings stored wherever the user wants. The options ~/projects or in the tree would be available as they are now.
To prevent the config dir getting too cluttered the session files can be kept only as long as the project remains in the recent projects list. (maybe needs a separate length setting, currently uses file_prefs.mru_length)
I will have some time next week, so this thread is to get ideas or objections sorted before then.
I like the idea of having workspaces/profiles, so that on startup (with option to not ask again) and then later through a menu, you can choose one of your profiles to load the settings like, indentation, long-line marker, colour scheme, etc.
Well, you can do it at startup with the -c option, changing it later isn't possible. I'm not offering to do this ATM.
IMO, projects should only store things like build commands, paths to project files, project meta-data, etc.
Yes, I included indentation and long line since those settings are likely (not guaranteed) to be consistent across a project but may be different between projects.
Sessions should be user-transparent things like window geometry, number of instances opened, last project/files, etc. and maybe should be workspace/profile-specific.
Yes, except for instances. Geany itself shouldn't handle those, thats the session manager's problem. But of course Gnome session manager is so broken that several current distros removed the option to use it, so I'm not sure when SM might work right.
I think this is how most other IDEs I've used handle the situation if I'm not mistaken.
Beware of Eclipsification creep when getting ideas from other IDEs. :)
Cheers Lex
On Tue, 1 Nov 2011 12:21:25 +1100 Lex Trotman elextr@gmail.com wrote:
On 1 November 2011 11:49, Matthew Brush mbrush@codebrainz.ca wrote:
On 11-10-31 04:50 PM, Lex Trotman wrote:
Sessions should be user-transparent things like window geometry, number of instances opened, last project/files, etc. and maybe should be workspace/profile-specific.
Yes, except for instances. Geany itself shouldn't handle those, thats the session manager's problem.
The current sm1 will handle that, unless you decide to separate the default session from geany.conf (which makes some sense btw).
But of course Gnome session manager is so broken that several current distros removed the option to use it, so I'm not sure when SM might work right.
Without a manager it won't work => same as the current non-sm Geany.
(OT: I wonder how many developers will stand a Gnome 3 or Unity interface, with the session support removed on top of that).
I will have some time next week, so this thread is to get ideas or objections sorted before then.
Applying #3312654 may help you a bit.
To prevent the config dir getting too cluttered the session files can be kept only as long as the project remains in the recent projects list. (maybe needs a separate length setting, currently uses file_prefs.mru_length)
At which point will you delete the unneeded session files?..
From what I've seen, the IDEs indeed keep the settings and file list
separate, but in the same directory. So if you delete the entire project directory, both go (if located there). And when you decide to delete a single project file, the session file is just next to it.
On 2 November 2011 05:19, Dimitar Zhekov dimitar.zhekov@gmail.com wrote:
On Tue, 1 Nov 2011 12:21:25 +1100 Lex Trotman elextr@gmail.com wrote:
On 1 November 2011 11:49, Matthew Brush mbrush@codebrainz.ca wrote:
On 11-10-31 04:50 PM, Lex Trotman wrote:
Sessions should be user-transparent things like window geometry, number of instances opened, last project/files, etc. and maybe should be workspace/profile-specific.
Yes, except for instances. Geany itself shouldn't handle those, thats the session manager's problem.
The current sm1 will handle that, unless you decide to separate the default session from geany.conf (which makes some sense btw).
Wasn't thinking of changing that ATM.
But of course Gnome session manager is so broken that several current distros removed the option to use it, so I'm not sure when SM might work right.
Without a manager it won't work => same as the current non-sm Geany.
(OT: I wonder how many developers will stand a Gnome 3 or Unity interface, with the session support removed on top of that).
Who knows when it will be fixed, the G* folks think with their egos, not their brains.
I will have some time next week, so this thread is to get ideas or objections sorted before then.
Applying #3312654 may help you a bit.
At first glance I'm not sure how, but will look closer.
To prevent the config dir getting too cluttered the session files can be kept only as long as the project remains in the recent projects list. (maybe needs a separate length setting, currently uses file_prefs.mru_length)
At which point will you delete the unneeded session files?..
When they fall off the recent projects list.
From what I've seen, the IDEs indeed keep the settings and file list separate, but in the same directory. So if you delete the entire project directory, both go (if located there). And when you decide to delete a single project file, the session file is just next to it.
The point is to get the session file out of the project tree, so no this defeats the purpose.
Cheers Lex
Am 01.11.2011 23:39, schrieb Lex Trotman:
The point is to get the session file out of the project tree, so no this defeats the purpose.
The point was to get the file list (i.e. what would be in that session file) out of VCS while keeping the project settings in.
This works with separate files even if in the same directory, since every reasonable VCS supports ignoring certain files (svn:ignore, .gitignore).
Best regards.
On Wed, 2 Nov 2011 09:39:48 +1100 Lex Trotman elextr@gmail.com wrote:
On 2 November 2011 05:19, Dimitar Zhekov dimitar.zhekov@gmail.com wrote:
On Tue, 1 Nov 2011 12:21:25 +1100 Lex Trotman elextr@gmail.com wrote:
At which point will you delete the unneeded session files?..
When they fall off the recent projects list.
For a small number of projects, that can keep the session file for years after the project is gone. Not a big deal though.
For many projects, well, you'd better max the recent count...
Am 01.11.2011 23:39, schrieb Lex Trotman:
The point is to get the session file out of the project tree, so no this defeats the purpose.
The point was to get the file list (i.e. what would be in that session file) out of VCS while keeping the project settings in.
This works with separate files even if in the same directory, since every reasonable VCS supports ignoring certain files (svn:ignore, .gitignore).
Of course. Personally I prefer to keep my projects clean from at least the object files and executables, but the IDEs do not hesistate to put there various .suo, .dsw, autogenerated .mak, entire directories even. Ignoring one more file (or not putting in on VCS in the first place) is hardly of any significance.
On 31/10/2011 23:50, Lex Trotman wrote:
Hi All,
One area of Geany has been annoying me for some time.
At the moment project settings mix session related settings and truely project settings. The session settings are really user specific whilst the project settings are project related. This means that if the project file is in VCS it keeps updating as the session settings change. You can turn session settings off, but then you lose that functionality.
The proposal is to separate these, with the session settings stored in the user config directory under a projects subdir and the project settings stored wherever the user wants. The options ~/projects or in the tree would be available as they are now.
IIUC the functionality would be the same but with different storage?
If so I'm not sure this would be worthwhile complexity. A starting project file could be hand edited and put in version control, for people to copy into their projects directory, i.e. not use the VC file itself.
On 2 November 2011 00:09, Nick Treleaven nick.treleaven@btinternet.com wrote:
On 31/10/2011 23:50, Lex Trotman wrote:
Hi All,
One area of Geany has been annoying me for some time.
At the moment project settings mix session related settings and truely project settings. The session settings are really user specific whilst the project settings are project related. This means that if the project file is in VCS it keeps updating as the session settings change. You can turn session settings off, but then you lose that functionality.
The proposal is to separate these, with the session settings stored in the user config directory under a projects subdir and the project settings stored wherever the user wants. The options ~/projects or in the tree would be available as they are now.
IIUC the functionality would be the same but with different storage?
If so I'm not sure this would be worthwhile complexity. A starting project file could be hand edited and put in version control, for people to copy into their projects directory, i.e. not use the VC file itself.
That doesn't keep the settings up to date as they need to change, thats the purpose of keeping it in the VCS.
Cheers Lex
Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
On 01/11/2011 22:24, Lex Trotman wrote:
On 2 November 2011 00:09, Nick Treleavennick.treleaven@btinternet.com wrote:
On 31/10/2011 23:50, Lex Trotman wrote:
IIUC the functionality would be the same but with different storage?
If so I'm not sure this would be worthwhile complexity. A starting project file could be hand edited and put in version control, for people to copy into their projects directory, i.e. not use the VC file itself.
That doesn't keep the settings up to date as they need to change, thats the purpose of keeping it in the VCS.
Yes, I just question whether it's worth supporting that.
[...]
Yes, I just question whether it's worth supporting that.
Well, it comes down to how groups and individuals work, you may not want to share projects settings, but I am part of groups who do.
Everything else is shared via VCS, so its annoying that one thing isn't.
Whats the problem with doing it?
Cheers Lex
Am 02.11.2011 13:10, schrieb Lex Trotman:
Well, it comes down to how groups and individuals work, you may not want to share projects settings, but I am part of groups who do.
Everything else is shared via VCS, so its annoying that one thing isn't.
Whats the problem with doing it?
Even if it's supported, people can chose to not share project settings by adding it to the ignore list. This way both is supported (which currently isn't the case).
Best regards.
On 02/11/2011 12:10, Lex Trotman wrote:
[...]
Yes, I just question whether it's worth supporting that.
Well, it comes down to how groups and individuals work, you may not want to share projects settings, but I am part of groups who do.
Everything else is shared via VCS, so its annoying that one thing isn't.
As I said, you can already share settings, you just can't easily synchronize them *if* they change.
Whats the problem with doing it?
It adds code complexity, plus backwards compatibility would make the code more ugly. Having all project settings stored in the same file is neater and easier to understand for users. It makes the manual more complex, maybe helping a little to obscure the important stuff.
I could bring up the Eclipse argument to say that worse is better. The more use cases Geany caters to, the more Eclipse-like it will be.
Am 02.11.2011 15:36, schrieb Nick Treleaven:
I could bring up the Eclipse argument to say that worse is better. The more use cases Geany caters to, the more Eclipse-like it will be.
I don't think we're any way near that. I also don't think we should stop improving (and making it possible to reasonably check-in project settings into VCS is an improvement in my book) in the fear of getting "Eclipse-like".
Having used eclipse, the worst part isn't its feature set. It's that it's written in Java...which arguably enables implementing the feature set in less time.
But let's not turn this into Eclipse-bashing thread :)
Best regards
[...]
Everything else is shared via VCS, so its annoying that one thing isn't.
As I said, you can already share settings, you just can't easily synchronize them *if* they change.
Whats the problem with doing it?
It adds code complexity, plus backwards compatibility would make the code more ugly. Having all project settings stored in the same file is neater and easier to understand for users. It makes the manual more complex, maybe helping a little to obscure the important stuff.
Nick, this argument is saying that Geany should never change or improve, if you want to stagnate just keep using 0.21 or your favourite version ...
Storing information with different uses (personal vs project) in the same file makes life harder for some users, all your arguments only relate to development issues not user issues.
I could bring up the Eclipse argument to say that worse is better. The more use cases Geany caters to, the more Eclipse-like it will be.
If there are going to be restrictions on what changes are made to Geany in the future then you need to put that proposal to the Geany community to consider how such decisions are made.
Cheers Lex
On 02/11/2011 23:03, Lex Trotman wrote:
[...]
Everything else is shared via VCS, so its annoying that one thing isn't.
As I said, you can already share settings, you just can't easily synchronize them *if* they change.
Whats the problem with doing it?
It adds code complexity, plus backwards compatibility would make the code more ugly. Having all project settings stored in the same file is neater and easier to understand for users. It makes the manual more complex, maybe helping a little to obscure the important stuff.
Nick, this argument is saying that Geany should never change or improve, if you want to stagnate just keep using 0.21 or your favourite version ...
Storing information with different uses (personal vs project) in the same file makes life harder for some users, all your arguments only relate to development issues not user issues.
First, I'd like to address these points, but I have actually thought of an alternative proposal which hopefully you might like and has much less impact on code. I'll start a new thread.
The point of arguing this out isn't especially for this feature, but for considering adding other features too.
My points are not just dev issues - users may want to backup their project file, which would be harder. Users need to read the manual, which would take longer to understand. Users might have to use buggier software, as more complex code is always more bug prone. Development issues become user issues indirectly.
I could bring up the Eclipse argument to say that worse is better. The more use cases Geany caters to, the more Eclipse-like it will be.
If there are going to be restrictions on what changes are made to Geany in the future then you need to put that proposal to the Geany community to consider how such decisions are made.
No, and also I'm not maintainer any more. But we do need to consider whether adding features is worth the maintenance and possible disruption.
Perhaps I was wrong to bring up Eclipse in this case, but I do think there are issues with your proposal which make it bad - as they're related to my proposal I'll deal with them in the new thread.
Regards, Nick
[...]
First, I'd like to address these points, but I have actually thought of an alternative proposal which hopefully you might like and has much less impact on code. I'll start a new thread.
Ok, all specific points transferred to that thread.
Just one note that the point of raising it in the ML before doing it was to see if anyone had better ideas for a solution. Geany needs more of that, as exhausting as it may be, rather than taking the first suggestion or patch or pre-implemented pull request. So thanks to Nick and all who contributed.
The point of arguing this out isn't especially for this feature, but for considering adding other features too.
Yes, its worthwhile discussing, but points like making the code overcomplicated etc need to be discussed in terms of a specifc implementation, as generalities they are meaningless (see below for more rant :).
My points are not just dev issues - users may want to backup their project file, which would be harder.
I don't understand how it would be harder, but anyway thats a specific.
Users need to read the manual, which would take longer to understand.
Again this is a specific, but I don't understand how having session info in a session file and project info in a project file is harder to understand, or that most users would even need to care.
Users might have to use buggier software, as more complex code is always more bug prone. Development issues become user issues indirectly.
Complicated isn't an absolute, it very much depends on experience, something you think is blindingly obvious I may find horrendously complex because I've never thought of that paradigm before. I agree that if the implementer considers it simple it is more likely to be right, but that doesn't guarantee that all maintainers will easily get their heads around it.
As an example Geany has many places where a short function then calls another short function which calls another short function, none of which are re-used. Personally I find this way of writing code less efficient and very hard to follow and understand as a whole, but others find it easier to think only in terms of each little piece. YMMV
[...]
No, and also I'm not maintainer any more. But we do need to consider whether adding features is worth the maintenance and possible disruption.
They should always be considered, I just disagree with you (what, never! :) and what I saw as overly general comments, ie "motherhood and apple pie".
[...]
Cheers Lex
On 11/03/2011 05:21 PM, Lex Trotman wrote:
[...]
First, I'd like to address these points, but I have actually thought of an alternative proposal which hopefully you might like and has much less impact on code. I'll start a new thread.
Ok, all specific points transferred to that thread.
Just one note that the point of raising it in the ML before doing it was to see if anyone had better ideas for a solution. Geany needs more of that, as exhausting as it may be, rather than taking the first suggestion or patch or pre-implemented pull request. So thanks to Nick and all who contributed.
This is just my take on the topic/issues in general and there's probably some things I haven't thought of but ...
It seems complicated having two separate project files for a single project, from a user POV. Wanting to check a project file into VCS seems like quite a common thing to do, and I might even do it for my own projects if the situation(s) in Geany were improved.
The way I see it, there's two issues currently: - session settings mixed with preference settings - things that should be able to be project-specific aren't
At a high level, the most sensible thing to do IMO, is to split out state/session information into a separate file (~/.config/geany/.geanysession) for Geany itself, and then mirror this in the project directory (~/project-under-vcs/foobar.geany and ~/project-under-vcs/.geanysession).
Basically everything in geany.conf that is "stateful" should not be in that file. Things like window geometry, open/recent files, sidebar pane position/etc. I guess the delimiter here would be things that the user changes explicitly vs. those they change implicitly. For example, the user doesn't change the sidebar position in a spin box, they just drag around the the splitter and expect Geany to respect this across runs/instances. They don't choose a list of files that should open next run using filechooserbuttons, they just open the files and expect them to open again if they were open last close. In this example, whether or not to re-open files on next run would be a preference.
Now take everything I just wrote and apply it to two other files; ~/my-project/projectfile.geany and ~/my-project/.geanysession. When a project opens, the project file and project session file "overlay" on top of the global/default settings/keys from ~/.config/geany/geany.conf and ~/.config/geany/.geanysession. Changes to the sidebar position for example would get written to ~/my-project/.geanysession and a change of indent type would go into ~/my-project/projectfile.geany.
It's not exactly trivial to code it, but really it's just about which GKeyFile gets read and written to, the guts of the files would be mostly the same as now, just the project prefs file would be a copy of geany.conf with a [project] group, and the state data would be split out into separate keyfiles for each. The logic to decide which file-set is simple: if project is open use the project files otherwise use the global/default files. The logic to decide which things are session vs. non-session could use the approach I mentioned above.
I would +1 something like I've described assuming two things:
- someone cares enough to do it (Lex?) - the code isn't just tacked onto the existing code, but instead the time is taken to clean up the existing code around this and implement it right so that all preferences and session data are split, and both are available in both projects and non-projects (except where it might not make sense, can't think of an example).
Doing this would solve both the code maintenance issue - by generally improving the whole project support and session support and related code - as well as solve the origin problems and in fact even improve Geany substantially, IMO.
The issue of backwards compatibility could be solved by adding a function that splits out the settings and writes them to the separate files if an "old-style" geany.conf or myproject.geany file are detected - probably confirming this with the user via a dialog or something. This shouldn't be terribly hard.
Sorry for the long explanation.
As an example Geany has many places where a short function then calls another short function which calls another short function, none of which are re-used. Personally I find this way of writing code less efficient and very hard to follow and understand as a whole, but others find it easier to think only in terms of each little piece. YMMV
Although it's somewhat off-topic, I have to agree. IMO, if a little chunk of code is only used in one spot, it shouldn't have it's own function, at least in most cases. I find it much easier to follow a slightly longer function than chasing the logic around little blobs of code scattered around the files.
Cheers, Matthew Brush
On Thu, 03 Nov 2011 20:03:02 -0700 Matthew Brush mbrush@codebrainz.ca wrote:
At a high level, the most sensible thing to do IMO, is to split out state/session information into a separate file (~/.config/geany/.geanysession) for Geany itself, and then mirror this in the project directory (~/project-under-vcs/foobar.geany and ~/project-under-vcs/.geanysession).
Why not ~/.config/geany/geany.session and /.../project.geanysession? I woudn't like geany to place a hidden file in the project directory, with no way to move it in the project file directory.
Now take everything I just wrote and apply it to two other files; ~/my-project/projectfile.geany and ~/my-project/.geanysession. When a project opens, the project file and project session file "overlay" on top of the global/default settings/keys from ~/.config/geany/geany.conf and ~/.config/geany/.geanysession. Changes to the sidebar position for example would get written to ~/my-project/.geanysession and a change of indent type would go into ~/my-project/projectfile.geany.
Would be nice, unless Edit -> Preferences decides to save the full set of options (even unchanged ones) in projectfile.geany. If I decide to change, say, the symbol list font, and have to do that for every project... well, the current configuration seems pretty reasonable.
But compating the two option sets, and (ideally) indicating the default (unchanged) settings with gray, as in the build dialog, goes beyound "not exactly trivial".
On 04/11/2011 00:21, Lex Trotman wrote:
Just one note that the point of raising it in the ML before doing it was to see if anyone had better ideas for a solution. Geany needs more of that, as exhausting as it may be, rather than taking the first suggestion or patch or pre-implemented pull request. So thanks to Nick and all who contributed.
Agree if people are likely to disagree on it, if the feature changes much code or causes implications for future additions etc. (Obviously we don't want to discuss everything as it would cause too much delay).
My points are not just dev issues - users may want to backup their project file, which would be harder.
I don't understand how it would be harder, but anyway thats a specific.
You would have to backup 2 files instead of 1 - not hard, *if* you understand this, but still more bother.
Users need to read the manual, which would take longer to understand.
Again this is a specific, but I don't understand how having session info in a session file and project info in a project file is harder to understand, or that most users would even need to care.
I didn't say harder, I said it would take longer. Anyway sometimes they do need to know (e.g. backup).
Users might have to use buggier software, as more complex code is always more bug prone. Development issues become user issues indirectly.
On 04/11/2011 00:21, Lex Trotman wrote:
Complicated isn't an absolute, it very much depends on experience, something you think is blindingly obvious I may find horrendously complex because I've never thought of that paradigm before. I agree that if the implementer considers it simple it is more likely to be right, but that doesn't guarantee that all maintainers will easily get their heads around it.
Complicated means more code changed than necessary (for sake of argument).
On 04/11/2011 00:21, Lex Trotman wrote:
Geany has many places where a short function then calls another short function which calls another short function, none of which are re-used. Personally I find this way of writing code less efficient and very hard to follow and understand as a whole, but others find it easier to think only in terms of each little piece. YMMV
Here may be somewhere where we disagree fundamentally, because:
* long functions cause bugs * too many variables in one place cause bugs
I'm sure there are statisics to back this up, it's well known in code analysis/reliability circles.
Breaking up logical tasks into functions is crucial to writing maintainable code, functions *are not just about code reuse*.
On Wed, Nov 9, 2011 at 3:40 AM, Nick Treleaven nick.treleaven@btinternet.com wrote:
On 04/11/2011 00:21, Lex Trotman wrote:
Geany has many places where a short function then calls another short function which calls another short function, none of which are re-used. Personally I find this way of writing code less efficient and very hard to follow and understand as a whole, but others find it easier to think only in terms of each little piece. YMMV
Here may be somewhere where we disagree fundamentally, because:
Probably :)
- long functions cause bugs
Rubbish, wrong functions cause bugs no matter how long, and being unable to see the whole logic leads to wrong design.
- too many variables in one place cause bugs
Thats what declarations in inner blocks are for :)
I'm sure there are statisics to back this up, it's well known in code analysis/reliability circles.
And there are also statistics that say the opposite, that breaking it up too far over complicates things and causes unreliability.
Breaking up logical tasks into functions is crucial to writing maintainable code, functions *are not just about code reuse*.
Yes, the key word here is *logical*, too big is bad and too small is bad, but what is too big and what is too small depends on the person and what the code is doing. It isn't a one size fits all.
I just noted that for me parts of Geany err on the too small side.
I guess we won't ever all agree on this, so we all have to be tolerant.
Cheers Lex
Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
On 11/08/2011 01:21 PM, Lex Trotman wrote:
On Wed, Nov 9, 2011 at 3:40 AM, Nick Treleaven nick.treleaven@btinternet.com wrote:
On 04/11/2011 00:21, Lex Trotman wrote:
Geany has many places where a short function then calls another short function which calls another short function, none of which are re-used. Personally I find this way of writing code less efficient and very hard to follow and understand as a whole, but others find it easier to think only in terms of each little piece. YMMV
Here may be somewhere where we disagree fundamentally, because:
Probably :)
I certainly do :)
- long functions cause bugs
Rubbish, wrong functions cause bugs no matter how long, and being unable to see the whole logic leads to wrong design.
+1
- too many variables in one place cause bugs
Can you elaborate on this assertion?
Thats what declarations in inner blocks are for :)
I'm sure there are statisics to back this up, it's well known in code analysis/reliability circles.
And there are also statistics that say the opposite, that breaking it up too far over complicates things and causes unreliability.
+1
Breaking up logical tasks into functions is crucial to writing maintainable code, functions *are not just about code reuse*.
But there's lots of places in Geany's code where logical tasks are actually further broken into many little sub-tasks (see below for an example), so small and trivial that the code would be a lot easier to follow if it was just in one place and achieved it's primary task.
Yes, the key word here is *logical*, too big is bad and too small is bad, but what is too big and what is too small depends on the person and what the code is doing. It isn't a one size fits all.
I just noted that for me parts of Geany err on the too small side.
I think you've summed that up pretty well.
Just to give an example, in encodings.c, there's a "logical" task towards the end of the code, which is to "guess encoding from a buffer and convert it". It starts out in encodings_convert_to_utf8_auto(), which is meant to do the task, but instead of that function doing what it's meant to do, it instead calls handle_buffer() to do it.
Now handle_buffer() is supposed to "guess encoding from a buffer and convert it" instead of the original function, so it first tries to detect the Unicode BOM, it calls encodings_scan_unicde_bom() which is fine, since it's logically a separate task and has a general usefulness (in fact in this case it's also re-used elsewhere, but pretend it wasn't).
Once the Unicode BOM is scanned, it continues on and if the encoding is to be forced, it calls yet another function, handle_forced_encoding() which could easily be inline in handle_buffer(). If not forcing the encoding, handle_buffer() will then call handle_encoding(), which actually does what handle_buffer() is supposed to do also; convert the encoding. handle_encoding() then calls out to the actual encoding conversion functions, with some logic/conditions around it. handle_encoding() could go into handle_buffer() as well, even though it's slightly on the longer side.
Continuing on, after handle_buffer() has called out to these other functions, it then shifts the buffer's data to overwrite the BOM bytes at the beginning, which is basically part of what handle_buffer() is supposed to be doing, but instead it calls yet another function named handle_bom() which is so short and simple that it should very much be in the handle_buffer().
Speaking only for myself, this is type of "chase the sub-logic" stuff is *so* confusing, and the problem becomes much worse when the "mini functions" are not directly near the "real function" in the file or worse yet in a whole other file. I'd have a *much* easier time reading a 200 line function that just does the one thing all these separate sub-functions combined are meant to do: automatically convert the encoding of the buffer.
I guess we won't ever all agree on this, so we all have to be tolerant.
Sadly, probably not :(
Cheers, Matthew Brush
On 09/11/2011 02:35, Matthew Brush wrote:
On 11/08/2011 01:21 PM, Lex Trotman wrote:
On Wed, Nov 9, 2011 at 3:40 AM, Nick Treleaven nick.treleaven@btinternet.com wrote:
Breaking up logical tasks into functions is crucial to writing maintainable code, functions *are not just about code reuse*.
But there's lots of places in Geany's code where logical tasks are actually further broken into many little sub-tasks (see below for an example), so small and trivial that the code would be a lot easier to follow if it was just in one place and achieved it's primary task.
I didn't mean to imply Geany's code is perfect, or that I usually write perfect code. I just wanted to draw attention to what I think the ideal is. Obviously I didn't explain it very well.
I guess we won't ever all agree on this, so we all have to be tolerant.
Sadly, probably not :(
Actually, I support the removal of functions which make code harder to follow, that is my only goal in discussing code style anyway.
E.g. I said I agreed about the removal of editor_lexer_get_type_keyword_idx().
On 08/11/2011 21:21, Lex Trotman wrote:
On Wed, Nov 9, 2011 at 3:40 AM, Nick Treleaven nick.treleaven@btinternet.com wrote:
On 04/11/2011 00:21, Lex Trotman wrote:
- long functions cause bugs
Rubbish, wrong functions cause bugs no matter how long, and being unable to see the whole logic leads to wrong design.
Long simple functions are Ok. Obscuring the logic is obviously bad. Sub-functions should be near the function that uses it if the sub-function is only used once.
- too many variables in one place cause bugs
Thats what declarations in inner blocks are for :)
That helps a bit, but the more things you have to keep track of the more likely you'll forget to initialize a variable that needs it or forget to free memory or other bugs.
I'm sure there are statisics to back this up, it's well known in code analysis/reliability circles.
And there are also statistics that say the opposite, that breaking it up too far over complicates things and causes unreliability.
Agree, I should have included some numbers in my points. It depends on context, but I would say a function longer than 15 lines may be too long. More than about 7 variables can get hard to keep track of everything.
On Fri, Nov 11, 2011 at 3:59 AM, Nick Treleaven nick.treleaven@btinternet.com wrote:
On 08/11/2011 21:21, Lex Trotman wrote:
On Wed, Nov 9, 2011 at 3:40 AM, Nick Treleaven nick.treleaven@btinternet.com wrote:
On 04/11/2011 00:21, Lex Trotman wrote:
- long functions cause bugs
Rubbish, wrong functions cause bugs no matter how long, and being unable to see the whole logic leads to wrong design.
Long simple functions are Ok. Obscuring the logic is obviously bad. Sub-functions should be near the function that uses it if the sub-function is only used once.
Agree with that, but just note that breaking out a sub-function should only happen if it is a logically separate operation. Breaking out something that is purely a part of a bigger operation just hides logic. For example if you can describe "this function furgles the foobar" then it *may* be a candidate for separation, but "this function does part of furgling the foobar" should not be considered separable. This applies to code re-use too, just because two things have substantially the same code, don't share a function unless it has a sensible describable purpose.
- too many variables in one place cause bugs
Thats what declarations in inner blocks are for :)
That helps a bit, but the more things you have to keep track of the more likely you'll forget to initialize a variable that needs it or forget to free memory or other bugs.
Agree, initialisation should be in declarations or immediately after if thats not possible.
I'm sure there are statisics to back this up, it's well known in code analysis/reliability circles.
And there are also statistics that say the opposite, that breaking it up too far over complicates things and causes unreliability.
Agree, I should have included some numbers in my points. It depends on context, but I would say a function longer than 15 lines may be too long. More than about 7 variables can get hard to keep track of everything.
We are both looking for the same result, more understandable and maintainable code.
But these specifics is where we disagree :)
Much of my experience is in the defence/aerospace sector which is mildly interested in reliability.
The coding standards used in this area tend not to be public, but a typical one that is available is the Joint Strike Fighter Air Vehicle coding rules http://www.research.att.com/~bs/JSF-AV-rules.pdf. This is for a system with rather more important reliability needs than Geany.
Whilst this is for C++, the general parts are applicable to C, in particular code size and complexity (3.2). Note that it separates the size from the complexity.
The size limit 200 logical SLOC is somewhat at the upper end of those I have seen, but 50 to 200 is the sort of range, and this is not required to be verified.
More important is the complexity, and this is required to be verified. This standard uses cyclomatic complexity as its measure, see Appendix A, but which measure is irrelevant.
The point is that the example in Appendix A is way under the complexity limits allowed, but I suspect would fail the Nick test as you outlined it here :)
I believe that your recommendations (for the best of motives) err dangerously on the too small side and would tend to decrease maintainability and reliability.
But as I said in the beginning YMMV :)
So long as we talk about it when we think it is a problem, and don't go unilaterally modifying code just to change this, it will be ok.
Cheers Lex