Hi, all,
I want to discuss how our FiF works, why, and possible improvements.
1. The FiF dialog is created programatically. Is there any reason for that, or simply nobody cared to XML-ize it? I'm not aware of anything that can be done gtk+ calls, but can't be done by loading a XML and less gtk+ calls.
Proposition: move the presentation to glade as much as possible, and fill the content with code, as in any normal program.
2. The all/project/custom combo box. This thing has 2 functions dedicated to it, and about 50 LOC in total. So what it does?
all - this is exactly the same as leaving the Files empty. custom - this is exactly the same as entering something in Files.
project - this is not exactly the same as placing the Project 'File patterns' in Files. It pastes them, all right, but then locks the field, so you can't add / remove / edit a pattern. And the only reason to lock them is "consistency": the 'project' choice must correspond to these patterns exactly. Well, if you do a Find, and then switch to 'custom', and then open the Files history, you will be able to edit the patterns after all... Last and least, 'project' grays the patterns, making them less visible.
Proposition: Remove the combo box. Add a paste icon on the right of files, above the Directory selection icon, and set it's tooltip to "Paste the project patterns, if any".
FiF checks if a project is open, but not whether it really contains 'File patterns' - RFC.
3. For non-recursive searches, grep does not allow a directory to be specified instead of file. To work around that, we read the directory, back-parse all --include=*.x patterns, and match them manually.
Proposition: grep -rl --include=*.c --exclude-dir=[^.]* --exclude-dir=.?* void . :)
We pass our Directory as a workind directory to spawn, and the recursive search already uses . as a grep FILE argument. --exclude-dir is supported since more than 5 years.
4. Pressing TAB in Files will take you directly to Directory, skipping the Files history arrow. Very nice, but a TAB in 'Search for' or Directory will select their history arrows, and Shift-TAB in Directory will select the Files arrow. Likewise, in the Replace dialog, TAB in 'Search for' will take you to 'Replace with', but another TAB will not skip to 'Use regular expressions'.
Proposition: TAB in FiF 'Search for' should skip to Directory, and TAB in Replace dialog 'Replace with' should skip to 'Use regular expressions'.
Or we should remove these hacks. Personally I find them useful. RFC.
5. Pressing ENTER in Directory does not activate Find.
Proposition: obvious.
6. search_find_in_files() checks if the grep command specified in Geany preferences exists in path.
Proposition: skip that and use G_SPAWN_SEARCH_PATH instead. If we want the explanation text "Cannot execute grep tool '%s' ...", show it after spawn fails - that way, it'll at least be correct.
Or, if we decide to leave the pre-check, at least change the text to "Cannot _find_ grep tool '%s' ..."
7. [ ] Extra options. Unchecking this is the same as clearing the text field, except that you can re-enable them later.
Proposition: Make 'Extra options' a label. If we want to keep the options, add a normal history. Personally I don't care about them.
That's all.
On 10 October 2013 07:01, Dimitar Zhekov dimitar.zhekov@gmail.com wrote:
Hi, all,
I want to discuss how our FiF works, why, and possible improvements.
- The FiF dialog is created programatically. Is there any reason for
that, or simply nobody cared to XML-ize it? I'm not aware of anything that can be done gtk+ calls, but can't be done by loading a XML and less gtk+ calls.
Well, not everybody likes Glade or XML, but why only FIF, why not all search dialogs?
Proposition: move the presentation to glade as much as possible, and fill the content with code, as in any normal program.
- The all/project/custom combo box. This thing has 2 functions
dedicated to it, and about 50 LOC in total. So what it does?
all - this is exactly the same as leaving the Files empty.
Yes would seem to be an alias of the delete button on the entry. Are you sure that the delete button is always guaranteed to be present, and not deprecated by GTK3 or removed by themes or ... something else that removes it?
custom - this is exactly the same as entering something in Files.
Well, its "Not Project", ie you need some way to switch away from using project settings, how good "Custom" is as a name is open.
project - this is not exactly the same as placing the Project 'File patterns' in Files. It pastes them, all right, but then locks the field, so you can't add / remove / edit a pattern. And the only reason to lock them is "consistency": the 'project' choice must correspond to these patterns exactly. Well, if you do a Find, and then switch to 'custom', and then open the Files history, you will be able to edit the patterns after all... Last and least, 'project' grays the patterns, making them less visible.
Make sure this functionality isn't important to the project plugin(s). I remember vaguely some significant discussion on the subject during one of the project plugin developments.
The fact that you can edit the paths when you have switched to custom is irrelevant, that doesn't save to the project.
Proposition: Remove the combo box. Add a paste icon on the right of files, above the Directory selection icon, and set it's tooltip to "Paste the project patterns, if any".
This is not the same functionality, choosing project on the combo is a sticky choice that follows the project settings, with this change the user would have to remember to paste when changing the project setting or after a custom search.
FiF checks if a project is open, but not whether it really contains 'File patterns' - RFC.
Which is the project's problem, not the search's. Search should not impose requirements on projects.
- For non-recursive searches, grep does not allow a directory to be
specified instead of file. To work around that, we read the directory, back-parse all --include=*.x patterns, and match them manually.
Proposition: grep -rl --include=*.c --exclude-dir=[^.]* --exclude-dir=.?* void . :)
We pass our Directory as a workind directory to spawn, and the recursive search already uses . as a grep FILE argument. --exclude-dir is supported since more than 5 years.
But not in POSIX grep. The hard coded grep options thing has always been a problem (eg see the Note in the manual). The grep setting in tools should accept a whole command like most of the other settings, probably with the usual %something substitutions.
- Pressing TAB in Files will take you directly to Directory, skipping
the Files history arrow. Very nice, but a TAB in 'Search for' or Directory will select their history arrows, and Shift-TAB in Directory will select the Files arrow. Likewise, in the Replace dialog, TAB in 'Search for' will take you to 'Replace with', but another TAB will not skip to 'Use regular expressions'.
Proposition: TAB in FiF 'Search for' should skip to Directory, and TAB in Replace dialog 'Replace with' should skip to 'Use regular expressions'.
Or we should remove these hacks. Personally I find them useful. RFC.
Don't use, don't care, but we probably should be consistent.
- Pressing ENTER in Directory does not activate Find.
Proposition: obvious.
I thought that was a PR, but I can't find it, nvm.
- search_find_in_files() checks if the grep command specified in Geany
preferences exists in path.
Proposition: skip that and use G_SPAWN_SEARCH_PATH instead. If we want the explanation text "Cannot execute grep tool '%s' ...", show it after spawn fails - that way, it'll at least be correct.
Agree.
Or, if we decide to leave the pre-check, at least change the text to "Cannot _find_ grep tool '%s' ..."
- [ ] Extra options. Unchecking this is the same as clearing the text
field, except that you can re-enable them later.
Proposition: Make 'Extra options' a label. If we want to keep the options, add a normal history. Personally I don't care about them.
In the absence of grep in tools prefs being a full command, this is where your --include and --exclude should be the default value. Personally I'd prefer the tools to be a full command.
That's all.
"All" :-)
Cheers Lex
-- E-gards: Jimmy _______________________________________________ Devel mailing list Devel@lists.geany.org https://lists.geany.org/cgi-bin/mailman/listinfo/devel
On Thu, 10 Oct 2013 09:54:37 +1100 Lex Trotman elextr@gmail.com wrote:
On 10 October 2013 07:01, Dimitar Zhekov dimitar.zhekov@gmail.com wrote:
Proposition: move the presentation to glade as much as possible, and fill the content with code, as in any normal program.
Well, not everybody likes Glade or XML, but why only FIF, why not all search dialogs?
I don't like XML either; only because I'm at the FiF dialog ATM.
- The all/project/custom combo box. This thing has 2 functions
dedicated to it, and about 50 LOC in total. So what it does?
all - this is exactly the same as leaving the Files empty.
Yes would seem to be an alias of the delete button on the entry. Are you sure that the delete button is always guaranteed to be present, and not deprecated by GTK3 or removed by themes or ... something else that removes it?
I'm not sure about the future of GTK+ at all, but that's another theme.
In the eventual future absence of the clear icon, you are free to clear the text with Delete/Backspace if it's selected, or with Ctrl+A and Delete/Backspace if it's not.
custom - this is exactly the same as entering something in Files.
Well, its "Not Project", ie you need some way to switch away from using project settings, how good "Custom" is as a name is open.
So it only exists because Project exists, and has no intrinsic value.
project - this is not exactly the same as placing the Project 'File patterns' in Files. It pastes them, all right, but then locks the field, so you can't add / remove / edit a pattern. And the only reason to lock them is "consistency": the 'project' choice must correspond to these patterns exactly. Well, if you do a Find, and then switch to 'custom', and then open the Files history, you will be able to edit the patterns after all... Last and least, 'project' grays the patterns, making them less visible.
Make sure this functionality isn't important to the project plugin(s). I remember vaguely some significant discussion on the subject during one of the project plugin developments.
If a plugin can access the field, it can simply fill it with something and lock it, without using an additional combo box.
Of course, if I change FiF, I will make sure that the plugins work.
The fact that you can edit the paths when you have switched to custom is irrelevant, that doesn't save to the project.
I could not understand this sentence, but agree about "not saving the project". It doesn't save "all" and "custom" either. :)
Proposition: Remove the combo box. Add a paste icon on the right of files, above the Directory selection icon, and set it's tooltip to "Paste the project patterns, if any".
This is not the same functionality, choosing project on the combo is a sticky choice that follows the project settings, with this change the user would have to remember to paste when changing the project setting or after a custom search.
And it also blocks editing the patterns. The most common starting point for specifying custom patterns does not work.
The "same functionality" can be achieved by using a check box, since the distinction between "all" and "custom" is nonsense. That'll still block editing, but at least won't be so clumsy.
Or we can check if the field contains the current project patterns, and fill it with the new project patters on switch. Big deal.
FiF checks if a project is open, but not whether it really contains 'File patterns' - RFC.
Which is the project's problem, not the search's. Search should not impose requirements on projects.
It doesn't, and I do not propose such a thing. It's just strange that "project" is disabled if there is no project, but enabled if there are no project paths.
- For non-recursive searches, grep does not allow a directory to be
specified instead of file. To work around that, we read the directory, back-parse all --include=*.x patterns, and match them manually.
Proposition: grep -rl --include=*.c --exclude-dir=[^.]* --exclude-dir=.?* void . :)
We pass our Directory as a workind directory to spawn, and the recursive search already uses . as a grep FILE argument. --exclude-dir is supported since more than 5 years.
But not in POSIX grep.
We are not using POSIX grep, it doesn't even support recursive search AFAIK. We are using GNU grep, and the above options work on our supported platforms, *NIX and Win32.
The hard coded grep options thing has always been a problem (eg see the Note in the manual). The grep setting in tools should accept a whole command like most of the other settings, probably with the usual %something substitutions.
"should" == "does not, and there's nobody willing to do it"?
And you won't be able to produce one --include= directive for each pattern in Files with the usual substitutions.
- Pressing ENTER in Directory does not activate Find.
Proposition: obvious.
I thought that was a PR, but I can't find it, nvm.
Maybe I was unclear. ENTER in 'Search for' or 'Files' activates the Find button. ENTER in Directory does not.
- [ ] Extra options. Unchecking this is the same as clearing the text
field, except that you can re-enable them later.
Proposition: Make 'Extra options' a label. If we want to keep the options, add a normal history. Personally I don't care about them.
In the absence of grep in tools prefs being a full command, this is where your --include and --exclude should be the default value.
I'll explain again. The only point of the checkbox is to be able to quickly disable the extra options, which is the same as clearing them, *except* that you can restore them quickly.
If we consider them so important as to have an extra interface element, why don't we use a normal history, and have several sets of options?
Am 11.10.2013 20:08, schrieb Dimitar Zhekov:
Proposition: Remove the combo box. Add a paste icon on the right of files, above the Directory selection icon, and set it's tooltip to "Paste the project patterns, if any".
This is not the same functionality, choosing project on the combo is a sticky choice that follows the project settings, with this change the user would have to remember to paste when changing the project setting or after a custom search.
And it also blocks editing the patterns. The most common starting point for specifying custom patterns does not work.
It does, although a bit awkward: Choose project first, then custom. The project patterns remain and become editable.
The "same functionality" can be achieved by using a check box, since the distinction between "all" and "custom" is nonsense. That'll still block editing, but at least won't be so clumsy.
Or we can check if the field contains the current project patterns, and fill it with the new project patters on switch. Big deal.
I dont feel strong about FiF, so do whatever you feel right. However I find the current combo box not that bad. The Project option is useful to automatically fill apply the project patterns. I do like your proposal of a "Paste project patterns" as well, but I fear that I need to press that repeatedly while now I can select project once and forget about it (even across restarts).
Best regards.
On Fri, 11 Oct 2013 20:18:50 +0200 Thomas Martitz thomas.martitz@student.htw-berlin.de wrote:
Am 11.10.2013 20:08, schrieb Dimitar Zhekov:
Proposition: Remove the combo box. Add a paste icon on the right of files, above the Directory selection icon, and set it's tooltip to "Paste the project patterns, if any".
And it also blocks editing the patterns. The most common starting point for specifying custom patterns does not work.
It does, although a bit awkward: Choose project first, then custom. The project patterns remain and become editable.
I didn't realize that, my mistake. Click-move-click to fill the patterns and another click-move-click to edit them seems to be longer than my mental prediction buffer... :)
I dont feel strong about FiF, so do whatever you feel right. However I find the current combo box not that bad. The Project option is useful to automatically fill apply the project patterns. I do like your proposal of a "Paste project patterns" as well, but I fear that I need to press that repeatedly while now I can select project once and forget about it (even across restarts).
Valid point, too. Geany doesn't save the current Files value, so after restart, we can't compare it against the Project patterns when switching to another project.
[...]
custom - this is exactly the same as entering something in Files.
Well, its "Not Project", ie you need some way to switch away from using project settings, how good "Custom" is as a name is open.
So it only exists because Project exists, and has no intrinsic value.
The key point of the "project" option is that the file patterns are stored in the *project* file not in the *user* preferences.
The project patterns are edited in the project preferences and the changed value appears in the FIF dialog field.
This is consistent with all the other places project settings override user settings, they need to be edited in the project prefs so the user understands that they are saved in the project file.
I'm not sure this is such a brilliant general UI decision, but its an attempt to ensure that users are not confused by the "complex" idea that one set of settings can override another, and that they know which set they are editing. I don't think this setting should deviate from the norm.
project - this is not exactly the same as placing the Project 'File patterns' in Files. It pastes them, all right, but then locks the field, so you can't add / remove / edit a pattern. And the only reason to lock them is "consistency": the 'project' choice must correspond to these patterns exactly. Well, if you do a Find, and then switch to 'custom', and then open the Files history, you will be able to edit the patterns after all... Last and least, 'project' grays the patterns, making them less visible.
Make sure this functionality isn't important to the project plugin(s). I remember vaguely some significant discussion on the subject during one of the project plugin developments.
If a plugin can access the field, it can simply fill it with something and lock it, without using an additional combo box.
Its not just the project plugins, the built-in project does it too, see above.
Of course, if I change FiF, I will make sure that the plugins work.
The fact that you can edit the paths when you have switched to custom is irrelevant, that doesn't save to the project.
I could not understand this sentence, but agree about "not saving the project". It doesn't save "all" and "custom" either. :)
Hopefully explained better above, sorry. Note that the combo box value and the "custom/non-project/whatever we want to call them" file patterns are meant to be saved in the user preferences, and it works for me.
Proposition: Remove the combo box. Add a paste icon on the right of files, above the Directory selection icon, and set it's tooltip to "Paste the project patterns, if any".
This is not the same functionality, choosing project on the combo is a sticky choice that follows the project settings, with this change the
user
would have to remember to paste when changing the project setting or
after
a custom search.
And it also blocks editing the patterns. The most common starting point for specifying custom patterns does not work.
Explained (hopefully) above.
The "same functionality" can be achieved by using a check box, since the distinction between "all" and "custom" is nonsense. That'll still block editing, but at least won't be so clumsy.
I wasn't meaning to argue *for* a combo, just the distinction between project and custom/non-project/whatever. As we agreed, "all" is redundant if the delete is available, so a "project" check box is fine by me so long as it does the same as the project setting on the combo.
Or we can check if the field contains the current project patterns, and fill it with the new project patters on switch. Big deal.
If "project" is selected copy whatever the project has in it. Blank means "everything" so it too has to be copied. There is really no need to check if its the same, just copy it :)
FiF checks if a project is open, but not whether it really contains 'File patterns' - RFC.
Which is the project's problem, not the search's. Search should not
impose
requirements on projects.
It doesn't, and I do not propose such a thing. It's just strange that "project" is disabled if there is no project, but enabled if there are no project paths.
See above, no patterns means "everything".
[...]
We are not using POSIX grep, it doesn't even support recursive search AFAIK. We are using GNU grep, and the above options work on our supported platforms, *NIX and Win32.
I understood that in the past there was an attempt to not limit this to GNU Grep.
Personally I would agree that it isn't worth supporting anything else, but Colomban needs to approve that decision and the documentation needs to be updated to note the limitation, and I guess the label on preferences->tools->grep should be "GNU grep" to make sure its understood thats all that is supported.
The hard coded grep options thing has always been a problem (eg see the Note in the manual). The grep setting in tools
should
accept a whole command like most of the other settings, probably with the usual %something substitutions.
"should" == "does not, and there's nobody willing to do it"?
Yep, or "nobody thought it was important enough to do it" :)
And you won't be able to produce one --include= directive for each pattern in Files with the usual substitutions.
By "usual" I just meant using the %, not what the actual possible substitutions would be.
- Pressing ENTER in Directory does not activate Find.
Proposition: obvious.
I thought that was a PR, but I can't find it, nvm.
Maybe I was unclear. ENTER in 'Search for' or 'Files' activates the Find button. ENTER in Directory does not.
Yes, you were clear, I just thought it had been raised, but anyway, yes it should be fixed to be consistent.
[...]
I'll explain again. The only point of the checkbox is to be able to quickly disable the extra options, which is the same as clearing them, *except* that you can restore them quickly.
If we consider them so important as to have an extra interface element, why don't we use a normal history, and have several sets of options?
"why don't" == "doesn't, and nobody cared enough to do it" ;-)
Cheers Lex
-- E-gards: Jimmy _______________________________________________ Devel mailing list Devel@lists.geany.org https://lists.geany.org/cgi-bin/mailman/listinfo/devel
On Sat, 12 Oct 2013 08:24:56 +1100 Lex Trotman elextr@gmail.com wrote:
The key point of the "project" option is that the file patterns are stored in the *project* file not in the *user* preferences.
I know, and never proposed to make patterns indicated as "project" editable.
Hopefully explained better above, sorry. Note that the combo box value and the "custom/non-project/whatever we want to call them" file patterns are meant to be saved in the user preferences, and it works for me.
I had a momentary lapse of reason, writing in the response to Martitz than the patterns are not saved, but that's of course wrong. The current value is saved.
Anyway, comparing the current FiF patterns against the current project patterns to decide where to paste the new project patterns when switching to another project will not be reliable. Close the project, restart, and there's no way to guess.
The "same functionality" can be achieved by using a check box, since the distinction between "all" and "custom" is nonsense. That'll still block editing, but at least won't be so clumsy.
I wasn't meaning to argue *for* a combo, just the distinction between project and custom/non-project/whatever. As we agreed, "all" is redundant if the delete is available, so a "project" check box is fine by me so long as it does the same as the project setting on the combo.
Since "project" does provide functionality, I'm changing the proposition: make it a check box. A single click will be enough to (un)lock it, and a double click (check-uncheck) will load the project patterns and enable editing.
(From UI standpoint, it may be better to make Files label a button, and the text between Files and Project on click - it'll look very similar to the current combo box. But I'm not a HIG expert.)
[...]
We are not using POSIX grep, it doesn't even support recursive search AFAIK. We are using GNU grep, and the above options work on our supported platforms, *NIX and Win32.
I understood that in the past there was an attempt to not limit this to GNU Grep.
Personally I would agree that it isn't worth supporting anything else, but Colomban needs to approve that decision and the documentation needs to be updated to note the limitation, and I guess the label on preferences->tools->grep should be "GNU grep" to make sure its understood thats all that is supported.
Yes. Unless there is another grep which has a --include options for patterns.
And you won't be able to produce one --include= directive for each pattern in Files with the usual substitutions.
By "usual" I just meant using the %, not what the actual possible substitutions would be.
--include=%p will simply produce --include=<all patterns>. We would need something like {--include=%p} to signify what text to repeat for each pattern.
I'll explain again. The only point of the checkbox is to be able to quickly disable the extra options, which is the same as clearing them, *except* that you can restore them quickly.
If we consider them so important as to have an extra interface element, why don't we use a normal history, and have several sets of options?
"why don't" == "doesn't, and nobody cared enough to do it" ;-)
:) On the same hand, we don't even save the Files history, and not even the current values of 'Search for' and 'Replace with'.
[...]
I wasn't meaning to argue *for* a combo, just the distinction between project and custom/non-project/whatever. As we agreed, "all" is
redundant
if the delete is available, so a "project" check box is fine by me so
long
as it does the same as the project setting on the combo.
Since "project" does provide functionality, I'm changing the proposition: make it a check box. A single click will be enough to (un)lock it, and a double click (check-uncheck) will load the project patterns and enable editing.
Sounds ok to me.
(From UI standpoint, it may be better to make Files label a button, and the text between Files and Project on click - it'll look very similar to the current combo box. But I'm not a HIG expert.)
Yes, any two-state widget would do, though I don't think changing the label is right, the field is always the file patterns, its just where they come from that changes. The advantage of the "Project" checkbox is we don't have to agree on a name for the non-project state :)
[...]
By "usual" I just meant using the %, not what the actual possible substitutions would be.
--include=%p will simply produce --include=<all patterns>. We would need something like {--include=%p} to signify what text to repeat for each pattern.
Such complications are probably part of why it hasn't been done :)
I'll explain again. The only point of the checkbox is to be able to quickly disable the extra options, which is the same as clearing them, *except* that you can restore them quickly.
If we consider them so important as to have an extra interface element, why don't we use a normal history, and have several sets of options?
"why don't" == "doesn't, and nobody cared enough to do it" ;-)
:) On the same hand, we don't even save the Files history, and not even the current values of 'Search for' and 'Replace with'.
I guess none of the developers do the same search/replace in multiple sessions very often, so saving settings and history hasn't been worth it.
Cheers Lex
--
E-gards: Jimmy _______________________________________________ Devel mailing list Devel@lists.geany.org https://lists.geany.org/cgi-bin/mailman/listinfo/devel
On 13-10-09 01:01 PM, Dimitar Zhekov wrote:
Hi, all,
I want to discuss how our FiF works, why, and possible improvements.
- The FiF dialog is created programatically. Is there any reason for
that, or simply nobody cared to XML-ize it? I'm not aware of anything that can be done gtk+ calls, but can't be done by loading a XML and less gtk+ calls.
Proposition: move the presentation to glade as much as possible, and fill the content with code, as in any normal program.
I have done this before in a branch, but just the Glade part, which AFAIK is basically unmergable now because of volatility of Glade XML file. What I did was I made one single Search dialog in Glade that always showed the widgets that are in common between the various search dialogs (find, replace, fif), and then the specific options for the search type were each in their own container widget so they could be hidden/shown depending on which search dialog needs to be shown. There's some extra code to setup the dialog to be shown/work for the correct search type but it probably dwarfs the amount of code removed by putting all of them in the Glade file into a single DRY dialog.
[snip]
- For non-recursive searches, grep does not allow a directory to be
specified instead of file. To work around that, we read the directory, back-parse all --include=*.x patterns, and match them manually.
Proposition: grep -rl --include=*.c --exclude-dir=[^.]* --exclude-dir=.?* void . :)
We pass our Directory as a workind directory to spawn, and the recursive search already uses . as a grep FILE argument. --exclude-dir is supported since more than 5 years.
Is there any reason we cannot just walk the directory/subdirectories ourselves and search the files using GRegex stuff? The pattern syntax would then be the exact same as the other search dialogs and it would save having to not only spawn a subprocess but also remove the dependency on grep (only a problem on Windows). Also, as a future optimization, if any of the files to search are open in Geany, we could search their document buffer directly from memory rather than having to do any file IO for them. GIO/GFile has all the stuff needed to walk a directory tree and open files both asynchronously and portably IIRC.
Cheers, Matthew Brush
[...]
- For non-recursive searches, grep does not allow a directory to be
specified instead of file. To work around that, we read the directory, back-parse all --include=*.x patterns, and match them manually.
Proposition: grep -rl --include=*.c --exclude-dir=[^.]* --exclude-dir=.?* void . :)
We pass our Directory as a workind directory to spawn, and the recursive search already uses . as a grep FILE argument. --exclude-dir is supported since more than 5 years.
Is there any reason we cannot just walk the directory/subdirectories ourselves and search the files using GRegex stuff? The pattern syntax would then be the exact same as the other search dialogs and it would save having to not only spawn a subprocess but also remove the dependency on grep (only a problem on Windows). Also, as a future optimization, if any of the files to search are open in Geany, we could search their document buffer directly from memory rather than having to do any file IO for them. GIO/GFile has all the stuff needed to walk a directory tree and open files both asynchronously and portably IIRC.
*Technically* the only issue I know of is that last time I looked PCRE was much less optimised than grep (partly because PCRE is more general) so searching a big tree of big files may be noticeably slower.
But this is more code that needs to be written, debugged and maintained. Pull requests (with an ongoing support pledge) are welcome.
The benefits of control and consistency (and as you pointed out on IRC to search the buffers not the files for open documents) may not outweigh the cost of doing it.
Cheers Lex
Cheers, Matthew Brush
______________________________**_________________ Devel mailing list Devel@lists.geany.org https://lists.geany.org/cgi-**bin/mailman/listinfo/develhttps://lists.geany.org/cgi-bin/mailman/listinfo/devel
On 13-10-09 07:15 PM, Lex Trotman wrote:
[...]
- For non-recursive searches, grep does not allow a directory to be
specified instead of file. To work around that, we read the directory, back-parse all --include=*.x patterns, and match them manually.
Proposition: grep -rl --include=*.c --exclude-dir=[^.]* --exclude-dir=.?* void . :)
We pass our Directory as a workind directory to spawn, and the recursive search already uses . as a grep FILE argument. --exclude-dir is supported since more than 5 years.
Is there any reason we cannot just walk the directory/subdirectories ourselves and search the files using GRegex stuff? The pattern syntax would then be the exact same as the other search dialogs and it would save having to not only spawn a subprocess but also remove the dependency on grep (only a problem on Windows). Also, as a future optimization, if any of the files to search are open in Geany, we could search their document buffer directly from memory rather than having to do any file IO for them. GIO/GFile has all the stuff needed to walk a directory tree and open files both asynchronously and portably IIRC.
*Technically* the only issue I know of is that last time I looked PCRE was much less optimised than grep (partly because PCRE is more general) so searching a big tree of big files may be noticeably slower.
But this is more code that needs to be written, debugged and maintained. Pull requests (with an ongoing support pledge) are welcome.
I'll gladly write and help maintain it if I don't have to bikeshed over using stupid old versions of GIO which might not have the features needed[1] to implement it properly without dirty hacks or #ifdef stuff :)
The benefits of control and consistency (and as you pointed out on IRC to search the buffers not the files for open documents) may not outweigh the cost of doing it.
Cheers, Matthew Brush
[1]: I haven't investigated deeply, the docs don't list "since" versions for some of the functions that would be used.
On Wed, 09 Oct 2013 18:43:41 -0700 Matthew Brush mbrush@codebrainz.ca wrote:
On 13-10-09 01:01 PM, Dimitar Zhekov wrote:
Proposition: move the presentation to glade as much as possible, and fill the content with code, as in any normal program.
I have done this before in a branch, but just the Glade part, which AFAIK is basically unmergable now because of volatility of Glade XML file. What I did was I made one single Search dialog in Glade that always showed the widgets that are in common between the various search dialogs (find, replace, fif), and then the specific options for the search type were each in their own container widget so they could be hidden/shown depending on which search dialog needs to be shown. There's some extra code to setup the dialog to be shown/work for the correct search type but it probably dwarfs the amount of code removed by putting all of them in the Glade file into a single DRY dialog.
Interesting... though I would probably have followed the current logic, simply moving the visual part to XML.
- For non-recursive searches, grep does not allow a directory to be
specified instead of file. To work around that, we read the directory, back-parse all --include=*.x patterns, and match them manually.
Proposition: grep -rl --include=*.c --exclude-dir=[^.]* --exclude-dir=.?* void . :)
We pass our Directory as a workind directory to spawn, and the recursive search already uses . as a grep FILE argument. --exclude-dir is supported since more than 5 years.
Is there any reason we cannot just walk the directory/subdirectories ourselves and search the files using GRegex stuff?
There's nobody willing to do it?..
GIO/GFile has all the stuff needed to walk a directory tree and open files both asynchronously and portably IIRC.
Looking at the "spawn" problems, which are realy GIOChannel set_flags and/or add_watch problems, I woudn't be so sure about that.
Also, the current FiF doesn't block Geany. If you want to keep that, you'll need a 2nd thread or an event source, and neither would be so easy to write.
On 13-10-11 11:23 AM, Dimitar Zhekov wrote:
On Wed, 09 Oct 2013 18:43:41 -0700 Matthew Brush mbrush@codebrainz.ca wrote:
On 13-10-09 01:01 PM, Dimitar Zhekov wrote:
Proposition: move the presentation to glade as much as possible, and fill the content with code, as in any normal program.
I have done this before in a branch, but just the Glade part, which AFAIK is basically unmergable now because of volatility of Glade XML file. What I did was I made one single Search dialog in Glade that always showed the widgets that are in common between the various search dialogs (find, replace, fif), and then the specific options for the search type were each in their own container widget so they could be hidden/shown depending on which search dialog needs to be shown. There's some extra code to setup the dialog to be shown/work for the correct search type but it probably dwarfs the amount of code removed by putting all of them in the Glade file into a single DRY dialog.
Interesting... though I would probably have followed the current logic, simply moving the visual part to XML.
- For non-recursive searches, grep does not allow a directory to be
specified instead of file. To work around that, we read the directory, back-parse all --include=*.x patterns, and match them manually.
Proposition: grep -rl --include=*.c --exclude-dir=[^.]* --exclude-dir=.?* void . :)
We pass our Directory as a workind directory to spawn, and the recursive search already uses . as a grep FILE argument. --exclude-dir is supported since more than 5 years.
Is there any reason we cannot just walk the directory/subdirectories ourselves and search the files using GRegex stuff?
There's nobody willing to do it?..
GIO/GFile has all the stuff needed to walk a directory tree and open files both asynchronously and portably IIRC.
Looking at the "spawn" problems, which are realy GIOChannel set_flags and/or add_watch problems, I woudn't be so sure about that.
OK, assuming some obscure bug from one part API doesn't somehow manifest itself in a seemingly completely different part of the API (IO Channels vs GIO/GFile).
Also, the current FiF doesn't block Geany. If you want to keep that, you'll need a 2nd thread or an event source, and neither would be so easy to write.
The GIO/GFile stuff I'm referring to is asynchronous.
Cheers, Matthew Brush
Am 10.10.2013 03:43, schrieb Matthew Brush:
On 13-10-09 01:01 PM, Dimitar Zhekov wrote:
Hi, all,
I want to discuss how our FiF works, why, and possible improvements.
- The FiF dialog is created programatically. Is there any reason for
that, or simply nobody cared to XML-ize it? I'm not aware of anything that can be done gtk+ calls, but can't be done by loading a XML and less gtk+ calls.
Proposition: move the presentation to glade as much as possible, and fill the content with code, as in any normal program.
I have done this before in a branch, but just the Glade part, which AFAIK is basically unmergable now because of volatility of Glade XML file. What I did was I made one single Search dialog in Glade that always showed the widgets that are in common between the various search dialogs (find, replace, fif), and then the specific options for the search type were each in their own container widget so they could be hidden/shown depending on which search dialog needs to be shown. There's some extra code to setup the dialog to be shown/work for the correct search type but it probably dwarfs the amount of code removed by putting all of them in the Glade file into a single DRY dialog.
I agree with the idea of moving it into glade files where it's more easily hackable. Try my patched glate from [1]. It should solve the volatile XML output.
Best regards.
Am 11.10.2013 21:30, schrieb Thomas Martitz:
I agree with the idea of moving it into glade files where it's more easily hackable. Try my patched glate from [1]. It should solve the volatile XML output.
[1]: https://github.com/kugel-/glade/tree/glade-3-8-fixes :)