Re: [Github-comments] [geany/geany] Make build_get/set_(current_)menu_item functions symmetrical (#1225)

2016-09-12 Thread Matthew Brush
> Maybe that one time could use get_next_build_cmd directly, and get_build_cmd > could lose the extra parameter. Yeah, that sounds like it might be better. There used to be 2 callers passing non-NULL but one was just deleted. -- You are receiving this because you are subscribed to this

Re: [Github-comments] [geany/geany] Make build_get/set_(current_)menu_item functions symmetrical (#1225)

2016-09-12 Thread elextr
One suggestion, `get_build_cmd` is just a convenience and not in the API and is only called once with a non-null `src`. Maybe that one time could use `get_next_build_cmd` directly, and `get_build_cmd` could lose the extra parameter. -- You are receiving this because you are subscribed to

Re: [Github-comments] [geany/geany] Make build_get/set_(current_)menu_item functions symmetrical (#1225)

2016-09-12 Thread elextr
As said on IRC, like the idea of putting the API number in `@since` annotations. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1225#issuecomment-246276341

Re: [Github-comments] [geany/geany] Make build_get/set_(current_)menu_item functions symmetrical (#1225)

2016-09-12 Thread Matthew Brush
@codebrainz pushed 1 commit. a3c9647 Add missing @since doc-comment tag (oops) -- You are receiving this because you are subscribed to this thread. View it on GitHub:

Re: [Github-comments] [geany/geany] Make build_get/set_(current_)menu_item functions symmetrical (#1225)

2016-09-12 Thread Matthew Brush
@codebrainz pushed 1 commit. 3ce7947 Use new build_get_current_source() function -- You are receiving this because you are subscribed to this thread. View it on GitHub:

Re: [Github-comments] [geany/geany] Make build_get/set_(current_)menu_item functions symmetrical (#1225)

2016-09-12 Thread Matthew Brush
Added two commits based on discussion. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1225#issuecomment-246267052

Re: [Github-comments] [geany/geany] Make build_get/set_(current_)menu_item functions symmetrical (#1225)

2016-09-12 Thread Matthew Brush
@codebrainz pushed 2 commits. 9b7303b Add function to get current build source b153777 Remove build_set_current_menu_item() function -- You are receiving this because you are subscribed to this thread. View it on GitHub:

Re: [Github-comments] [geany/geany] Make build_get/set_(current_)menu_item functions symmetrical (#1225)

2016-09-12 Thread elextr
No, you should always know what you are setting, randomly overwriting a command that might have been set by a user, or the filetype, or the project is wrong. You want to get the current command to see what will be executed. Perhaps it would have been better to return the source instead of the

Re: [Github-comments] [geany/geany] Make build_get/set_(current_)menu_item functions symmetrical (#1225)

2016-09-11 Thread Matthew Brush
> You never want to set the current build command, thats the point, why do you > want to set a command in an unknown source? If you want to set the build command which is currently active, otherwise you have to figure out if a project is open, for example, and then set that build command

Re: [Github-comments] [geany/geany] Make build_get/set_(current_)menu_item functions symmetrical (#1225)

2016-09-11 Thread elextr
You never want to set the current build command, thats the point, why do you want to set a command in an unknown source? "Current" is a state, not a thing, it may be different after the next user input. You then don't know what you set. On 12 September 2016 at 15:47, Matthew Brush

Re: [Github-comments] [geany/geany] Make build_get/set_(current_)menu_item functions symmetrical (#1225)

2016-09-11 Thread Matthew Brush
> That set_current functionality should not be added is the main point. It's the only way to set the current build command, that's the rationale :) An alternative would be a function like `build_get_current_source()` to get the current build source and then use that the set it. -- You are

Re: [Github-comments] [geany/geany] Make build_get/set_(current_)menu_item functions symmetrical (#1225)

2016-09-11 Thread elextr
The complexity (or not) isn't the main point. That `set_current` functionality should not be added is the main point. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub:

Re: [Github-comments] [geany/geany] Make build_get/set_(current_)menu_item functions symmetrical (#1225)

2016-09-11 Thread Matthew Brush
> Looks unnecessarily complex. Believe it or not, the code is actually simplified since it removes an unused function and refactors an existing function to be in terms of another function instead of duplicating code. But yeah, the diff looks complex at a glance due to the way diffs work. --

Re: [Github-comments] [geany/geany] Make build_get/set_(current_)menu_item functions symmetrical (#1225)

2016-09-11 Thread elextr
Looks unnecessarily complex. Shouldn't need all the changes to the internals. Get any command is a good idea, yes. To do that I suggest you modify `build_get_menu_item()` which is not in the API and not used in Geany. Just add the field parameter, and the switch statement from

[Github-comments] [geany/geany] Make build_get/set_(current_)menu_item functions symmetrical (#1225)

2016-09-11 Thread Matthew Brush
So plugins can set the current command rather than only get it, as well as get a specific command rather than only set it. __Note:__ This needs real code review since I had a hard time following the code in `build.c`, I'm not 100% confident I implemented this correctly. Will be testing with a