<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On 10 October 2013 07:01, Dimitar Zhekov <span dir="ltr"><<a href="mailto:dimitar.zhekov@gmail.com" target="_blank">dimitar.zhekov@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi, all,<br>
<br>
I want to discuss how our FiF works, why, and possible improvements.<br>
<br>
1. The FiF dialog is created programatically. Is there any reason for<br>
that, or simply nobody cared to XML-ize it? I'm not aware of anything<br>
that can be done gtk+ calls, but can't be done by loading a XML and<br>
less gtk+ calls.<br></blockquote><div><br></div><div>Well, not everybody likes Glade or XML, but why only FIF, why not all search dialogs?</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Proposition: move the presentation to glade as much as possible, and<br>
fill the content with code, as in any normal program.<br>
<br>
<br>
2. The all/project/custom combo box. This thing has 2 functions<br>
dedicated to it, and about 50 LOC in total. So what it does?<br>
<br>
all - this is exactly the same as leaving the Files empty.<br></blockquote><div><br></div><div>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?</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
custom - this is exactly the same as entering something in Files.</blockquote><div><br></div><div>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.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
project - this is not exactly the same as placing the Project 'File<br>
patterns' in Files. It pastes them, all right, but then locks the<br>
field, so you can't add / remove / edit a pattern. And the only reason<br>
to lock them is "consistency": the 'project' choice must correspond to<br>
these patterns exactly. Well, if you do a Find, and then switch to<br>
'custom', and then open the Files history, you will be able to edit<br>
the patterns after all... Last and least, 'project' grays the patterns,<br>
making them less visible.<br></blockquote><div><br></div><div>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.</div>
<div><br></div><div>The fact that you can edit the paths when you have switched to custom is irrelevant, that doesn't save to the project.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Proposition:<br>
Remove the combo box.<br>
Add a paste icon on the right of files, above the Directory selection<br>
icon, and set it's tooltip to "Paste the project patterns, if any".<br></blockquote><div><br></div><div>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.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
FiF checks if a project is open, but not whether it really contains<br>
'File patterns' - RFC.<br></blockquote><div><br></div><div>Which is the project's problem, not the search's. Search should not impose requirements on projects.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
<br>
3. For non-recursive searches, grep does not allow a directory to be<br>
specified instead of file. To work around that, we read the directory,<br>
back-parse all --include=*.x patterns, and match them manually.<br>
<br>
Proposition: grep -rl --include=*.c --exclude-dir=[^.]*<br>
--exclude-dir=.?* void . :)<br>
<br>
We pass our Directory as a workind directory to spawn, and the<br>
recursive search already uses . as a grep FILE argument.<br>
--exclude-dir is supported since more than 5 years.<br></blockquote><div><br></div><div>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.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
<br>
4. Pressing TAB in Files will take you directly to Directory, skipping<br>
the Files history arrow. Very nice, but a TAB in 'Search for' or<br>
Directory will select their history arrows, and Shift-TAB in Directory<br>
will select the Files arrow. Likewise, in the Replace dialog, TAB in<br>
'Search for' will take you to 'Replace with', but another TAB will not<br>
skip to 'Use regular expressions'.<br>
<br>
Proposition: TAB in FiF 'Search for' should skip to Directory, and TAB<br>
in Replace dialog 'Replace with' should skip to 'Use regular<br>
expressions'.<br>
<br>
Or we should remove these hacks. Personally I find them useful. RFC.<br></blockquote><div><br></div><div>Don't use, don't care, but we probably should be consistent.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
<br>
5. Pressing ENTER in Directory does not activate Find.<br>
<br>
Proposition: obvious.<br></blockquote><div><br></div><div>I thought that was a PR, but I can't find it, nvm.</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
<br>
6. search_find_in_files() checks if the grep command specified in Geany<br>
preferences exists in path.<br>
<br>
Proposition: skip that and use G_SPAWN_SEARCH_PATH instead. If we want<br>
the explanation text "Cannot execute grep tool '%s' ...", show it after<br>
spawn fails - that way, it'll at least be correct.<br></blockquote><div><br></div><div>Agree.</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Or, if we decide to leave the pre-check, at least change the text to<br>
"Cannot _find_ grep tool '%s' ..."</blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
7. [ ] Extra options. Unchecking this is the same as clearing the text<br>
field, except that you can re-enable them later.<br>
<br>
Proposition:<br>
Make 'Extra options' a label.<br>
If we want to keep the options, add a normal history. Personally I<br>
don't care about them.<br></blockquote><div><br></div><div>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.</div>
<div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
<br>
That's all.<br>
<span class="HOEnZb"><font color="#888888"><br></font></span></blockquote><div><br></div><div>"All" :-)</div><div><br></div><div>Cheers</div><div>Lex</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class="HOEnZb"><font color="#888888">
--<br>
E-gards: Jimmy<br>
_______________________________________________<br>
Devel mailing list<br>
<a href="mailto:Devel@lists.geany.org">Devel@lists.geany.org</a><br>
<a href="https://lists.geany.org/cgi-bin/mailman/listinfo/devel" target="_blank">https://lists.geany.org/cgi-bin/mailman/listinfo/devel</a><br>
</font></span></blockquote></div><br></div></div>