Hi,
Recent bug report #3390435 [1] remembered me about per-filetype indentation settings. This would remove hard-coded settings we have for Makefile and Fortran 77, moving the setting to the filetype definitions, as well as allowing further per-filetype tweaking -- e.g. we could imagine that somebody would like to have Python set to "4 spaces" to follow PEP8.
I have come with an initial implementation and would like your wise criticism and design opinions.
A few trivial questions: * should the settings be "indent_type" and "indent_width" under [settings] rather than adding a new section? * do you think it's sensible to reset the indentation settings when switch from a filetype that had specific settings? * do you think the filetype-specific settings should override the auto-detection? (the second patch makes auto-detection prevail)
So, what do you think of the feature, the implementation, the missing parts, the universe and everything?
Cheers, Colomban
[1] https://sourceforge.net/tracker/?func=detail&atid=787791&aid=3390435...
Hi Colomban,
On 14 August 2011 06:17, Colomban Wendling lists.ban@herbesfolles.org wrote:
Hi,
Recent bug report #3390435 [1] remembered me about per-filetype indentation settings. This would remove hard-coded settings we have for Makefile and Fortran 77, moving the setting to the filetype definitions, as well as allowing further per-filetype tweaking -- e.g. we could imagine that somebody would like to have Python set to "4 spaces" to follow PEP8.
Good idea.
I have come with an initial implementation and would like your wise criticism and design opinions.
I havn't had the chance to try it, but by my (questionable) inspection it looks ok.
A few trivial questions: * should the settings be "indent_type" and "indent_width" under [settings] rather than adding a new section?
I don't see much difference, so keep your existing implementation.
* do you think it's sensible to reset the indentation settings when switch from a filetype that had specific settings?
Settings should take the value of the most specific setting present, ie individual file specific, user filetype specific, system filetype specific, user general preference, system general setting/default value. If something changes in that sequence like changing the filetype then yes it should change.
* do you think the filetype-specific settings should override the auto-detection? (the second patch makes auto-detection prevail)
Well if they do and there is a filetype setting for every filetype, how do the detected settings get used? Would need a menu option and possibly an option on the file open dialog to say use detected settings. And a preference of course.
So, what do you think of the feature, the implementation, the missing parts, the universe and everything?
42 :-)
Cheers Lex
PS minor English correction for the documentation, "allows to define" S/B "allows definition of"
Le 14/08/2011 06:54, Lex Trotman a écrit :
Hi Colomban,
On 14 August 2011 06:17, Colomban Wendling lists.ban@herbesfolles.org wrote:
Hi,
Recent bug report #3390435 [1] remembered me about per-filetype indentation settings. This would remove hard-coded settings we have for Makefile and Fortran 77, moving the setting to the filetype definitions, as well as allowing further per-filetype tweaking -- e.g. we could imagine that somebody would like to have Python set to "4 spaces" to follow PEP8.
Good idea.
I have come with an initial implementation and would like your wise criticism and design opinions.
I havn't had the chance to try it, but by my (questionable) inspection it looks ok.
At least another pair of eyes looked at the code and didn't see anything obvious :)
A few trivial questions:
- should the settings be "indent_type" and "indent_width" under
[settings] rather than adding a new section?
I don't see much difference, so keep your existing implementation.
Actually, I think using sections rather than prefixing the key's name is probably better (that's the reason I did so in the first place), but OTOH all our current settings uses prefixed names in the [settings] section... anyway probably not a big deal.
- do you think it's sensible to reset the indentation settings when
switch from a filetype that had specific settings?
Settings should take the value of the most specific setting present, ie individual file specific, user filetype specific, system filetype specific, user general preference, system general setting/default value. If something changes in that sequence like changing the filetype then yes it should change.
That was my thinking too. The only thing we may want to add is the ability to re-do the auto-detection in such cases (or remember the previous values, but it looks quite overkill).
- do you think the filetype-specific settings should override the
auto-detection? (the second patch makes auto-detection prevail)
Well if they do and there is a filetype setting for every filetype, how do the detected settings get used?
They wouldn't, that's the reason I did the 2nd patch :)
Would need a menu option and possibly an option on the file open dialog to say use detected settings. And a preference of course.
Well, we already have a pref to chose whether to auto-detect or not (actually, two: one for type and one for width). So either we add two more filetype settings to choose whether auto-detection can override the defined values, or we leave this to the current global preferences and don't override them at all.
PS minor English correction for the documentation, "allows to define" S/B "allows definition of"
Fixed, thanks :)
Cheers, Colomban
Hi Colomban,
[...]
Settings should take the value of the most specific setting present, ie individual file specific, user filetype specific, system filetype specific, user general preference, system general setting/default value. If something changes in that sequence like changing the filetype then yes it should change.
That was my thinking too. The only thing we may want to add is the ability to re-do the auto-detection in such cases (or remember the previous values, but it looks quite overkill).
I have thought some more on this and I have changed my mind, I am now convinced that the settings should not change for open files unless the user deliberately changes them.
The reason is that changing settings does not change the content of the open files. So if the settings can change they can become different to the file's existing content, and that would lead to files having mixed indentations, something that is frowned on (or totally wrong for offside languages like Python).
Instead the hierarchy I outlined above should set the indentation settings when opening files unless the preference has been set to use auto detection. The hierarchy should also be used to set the indentation settings for new files the first time the filetype is set (provided the user hasn't manually set the indentation before setting the filetype).
I would suggest adding an indentation setting combo box to the open file dialog (like the encoding one) with the options:
1. to set specific indentation settings (and the selectors for those settings of course) 2. to use autodetect 3. to use whichever the hierarchy chooses based on prefs or filetypes (this should of course show what it will be and potentially the source)
The default would be 2 or 3 depending on the "use autodetect" preference.
The user must always be allowed to change the settings as the user has the menu items to change the content if required, or if autodetect gets it wrong, or its just a file with a different indentation standard, they do exist :-)
* do you think the filetype-specific settings should override the auto-detection? (the second patch makes auto-detection prevail)
Well if they do and there is a filetype setting for every filetype, how do the detected settings get used?
They wouldn't, that's the reason I did the 2nd patch :)
That was what I thought :)
[...]
Cheers Lex
Le 15/08/2011 14:00, Lex Trotman a écrit :
Hi Colomban,
[...]
Settings should take the value of the most specific setting present, ie individual file specific, user filetype specific, system filetype specific, user general preference, system general setting/default value. If something changes in that sequence like changing the filetype then yes it should change.
That was my thinking too. The only thing we may want to add is the ability to re-do the auto-detection in such cases (or remember the previous values, but it looks quite overkill).
I have thought some more on this and I have changed my mind, I am now convinced that the settings should not change for open files unless the user deliberately changes them.
The reason is that changing settings does not change the content of the open files. So if the settings can change they can become different to the file's existing content, and that would lead to files having mixed indentations, something that is frowned on (or totally wrong for offside languages like Python).
Instead the hierarchy I outlined above should set the indentation settings when opening files unless the preference has been set to use auto detection. The hierarchy should also be used to set the indentation settings for new files the first time the filetype is set (provided the user hasn't manually set the indentation before setting the filetype).
Makes sense.
So if setting the filetype for a file that had no one set [1], apply the indent settings hierarchy, otherwise do nothing. Maybe we should do the same as on open, so also do auto-detection?
[1] does assuming None filetype means "no user choice" OK, or should we track whether the user chosen None?
I would suggest adding an indentation setting combo box to the open file dialog (like the encoding one) with the options:
- to set specific indentation settings (and the selectors for those
settings of course) 2. to use autodetect 3. to use whichever the hierarchy chooses based on prefs or filetypes (this should of course show what it will be and potentially the source)
The default would be 2 or 3 depending on the "use autodetect" preference.
The user must always be allowed to change the settings as the user has the menu items to change the content if required, or if autodetect gets it wrong, or its just a file with a different indentation standard, they do exist :-)
I'm not sure a setting in the file open dialogue makes much sense. I doubt this setting would be used often, and the user can easily change the setting once the file is actually opened. Do you really think it's worth adding one more option to file opening?
Cheers, Colomban
[...]
Makes sense.
So if setting the filetype for a file that had no one set [1], apply the indent settings hierarchy, otherwise do nothing. Maybe we should do the same as on open, so also do auto-detection?
If the indentation setting is set on open, it is not possible to have a file open without indentation settings. After opening the only time it should change is if the user sets it, and then it should go to what the user specifies.
[1] does assuming None filetype means "no user choice" OK, or should we track whether the user chosen None?
There is always some setting, even if its a default coded in Geany. IIUC thats what happens now if you create a new file.
I would suggest adding an indentation setting combo box to the open file dialog (like the encoding one) with the options:
- to set specific indentation settings (and the selectors for those
settings of course) 2. to use autodetect 3. to use whichever the hierarchy chooses based on prefs or filetypes (this should of course show what it will be and potentially the source)
The default would be 2 or 3 depending on the "use autodetect" preference.
The user must always be allowed to change the settings as the user has the menu items to change the content if required, or if autodetect gets it wrong, or its just a file with a different indentation standard, they do exist :-)
I'm not sure a setting in the file open dialogue makes much sense. I doubt this setting would be used often, and the user can easily change the setting once the file is actually opened. Do you really think it's worth adding one more option to file opening?
Well in the new scheme its the only time the user can tell Geany to use detect when its not the usual preference, unless we add it to the set indent menu, which is probably the better option.
Cheers Lex
Le 21/08/2011 05:22, Lex Trotman a écrit :
[...]
Makes sense.
So if setting the filetype for a file that had no one set [1], apply the indent settings hierarchy, otherwise do nothing. Maybe we should do the same as on open, so also do auto-detection?
If the indentation setting is set on open, it is not possible to have a file open without indentation settings. After opening the only time it should change is if the user sets it, and then it should go to what the user specifies.
But when creating a new file. The user will set the filetype, but will also probably want her filetypes indentation settings to be set since she didn't chose any at this point -- well, assuming she didn't chose indentation settings first, so checking this too.
[1] does assuming None filetype means "no user choice" OK, or should we track whether the user chosen None?
There is always some setting, even if its a default coded in Geany. IIUC thats what happens now if you create a new file.
What I meant is that we need to track whether the user set a fietype if we don't assume "filetype None" == "the use did not choice". Both are possible, assumption is cheaper, the other probably a little better.
I would suggest adding an indentation setting combo box to the open file dialog (like the encoding one) with the options:
- to set specific indentation settings (and the selectors for those
settings of course) 2. to use autodetect 3. to use whichever the hierarchy chooses based on prefs or filetypes (this should of course show what it will be and potentially the source)
The default would be 2 or 3 depending on the "use autodetect" preference.
The user must always be allowed to change the settings as the user has the menu items to change the content if required, or if autodetect gets it wrong, or its just a file with a different indentation standard, they do exist :-)
I'm not sure a setting in the file open dialogue makes much sense. I doubt this setting would be used often, and the user can easily change the setting once the file is actually opened. Do you really think it's worth adding one more option to file opening?
Well in the new scheme its the only time the user can tell Geany to use detect when its not the usual preference, unless we add it to the set indent menu, which is probably the better option.
Well, if the user haven't chosen auto-detection in the setting it's probably she don't want it. But I agree that we probably should have a "reset indentation settings" or something in the document menu, which would basically call document_apply_indent_settings().
Cheers, Colomban
[...]
If the indentation setting is set on open, it is not possible to have a file open without indentation settings. After opening the only time it should change is if the user sets it, and then it should go to what the user specifies.
But when creating a new file. The user will set the filetype, but will also probably want her filetypes indentation settings to be set since she didn't chose any at this point -- well, assuming she didn't chose indentation settings first, so checking this too.
Sorry, you are right, I didn't read it properly and confused setting filetype and setting indentation.
[1] does assuming None filetype means "no user choice" OK, or should we track whether the user chosen None?
There is always some setting, even if its a default coded in Geany. IIUC thats what happens now if you create a new file.
What I meant is that we need to track whether the user set a fietype if we don't assume "filetype None" == "the use did not choice". Both are possible, assumption is cheaper, the other probably a little better.
As above.
[...]
Well in the new scheme its the only time the user can tell Geany to use detect when its not the usual preference, unless we add it to the set indent menu, which is probably the better option.
Well, if the user haven't chosen auto-detection in the setting it's probably she don't want it.
Well if uses autodetect which is in preferences only, it isn't convenient to change it for an occasional document (and then go back to preferences and change it back), thats what I meant for opening a document that wasn't the "usual" setting.
But I agree that we probably should have a
"reset indentation settings" or something in the document menu, which would basically call document_apply_indent_settings().
No because that still depends on the preference, so the user still has to go change the preference menu->reset it then go change the preference back. IMHO the menu should offer the user explicit control, set spaces, set tabs, detect, set width, not depend on a preference that the user has probably forgotten about :-)
Cheers Lex
Le 22/08/2011 03:28, Lex Trotman a écrit :
[...]
Well in the new scheme its the only time the user can tell Geany to use detect when its not the usual preference, unless we add it to the set indent menu, which is probably the better option.
Well, if the user haven't chosen auto-detection in the setting it's probably she don't want it.
Well if uses autodetect which is in preferences only, it isn't convenient to change it for an occasional document (and then go back to preferences and change it back), thats what I meant for opening a document that wasn't the "usual" setting.
This means that *before* opening a document, the user knows she doesn't want her default settings nor knows what she wants exactly?
But I agree that we probably should have a
"reset indentation settings" or something in the document menu, which would basically call document_apply_indent_settings().
No because that still depends on the preference, so the user still has to go change the preference menu->reset it then go change the preference back. IMHO the menu should offer the user explicit control, set spaces, set tabs, detect, set width, not depend on a preference that the user has probably forgotten about :-)
That makes sense, but I also think that if the user *changes* the indentation settings she probably don't need auto-detect since she probably knows what she wants. However it'd be easy to add "auto-detect" to indentation type and width selection menus -- though indentation width detection doesn't work for tabs-based indentation for now.
So, to summarize a little:
1) Add per-filetype indentation width and type;
2) When setting the filetype for the first time (file open and unpon initial manual filetype selection), use, in order: a) auto-detect (if preferred) b) filetype setting c) user global settings d) system global settings
3) when changing the filetype for the second (or more) time, don't touch indentation settings.
Sounds good?
Cheers, Colomban
On 22 August 2011 22:57, Colomban Wendling lists.ban@herbesfolles.org wrote:
Le 22/08/2011 03:28, Lex Trotman a écrit :
[...]
Well in the new scheme its the only time the user can tell Geany to use detect when its not the usual preference, unless we add it to the set indent menu, which is probably the better option.
Well, if the user haven't chosen auto-detection in the setting it's probably she don't want it.
Well if uses autodetect which is in preferences only, it isn't convenient to change it for an occasional document (and then go back to preferences and change it back), thats what I meant for opening a document that wasn't the "usual" setting.
This means that *before* opening a document, the user knows she doesn't want her default settings nor knows what she wants exactly?
Which is why I suggested that putting it in the menu would be better.
But I agree that we probably should have a
"reset indentation settings" or something in the document menu, which would basically call document_apply_indent_settings().
No because that still depends on the preference, so the user still has to go change the preference menu->reset it then go change the preference back. IMHO the menu should offer the user explicit control, set spaces, set tabs, detect, set width, not depend on a preference that the user has probably forgotten about :-)
That makes sense, but I also think that if the user *changes* the indentation settings she probably don't need auto-detect since she probably knows what she wants. However it'd be easy to add "auto-detect" to indentation type and width selection menus -- though indentation width detection doesn't work for tabs-based indentation for now.
The autodetect entry is to provide a way to return it to autodected values after I mucked it up by fiddling and can't remember where I started :-)
So, to summarize a little:
Add per-filetype indentation width and type;
When setting the filetype for the first time (file open and unpon
initial manual filetype selection), use, in order: a) auto-detect (if preferred)
or not new file (can't detect a non-existent file :-)
b) filetype setting c) user global settings d) system global settings
- when changing the filetype for the second (or more) time, don't touch
indentation settings.
Sounds good?
Yep.
Cheers Lex
Le 23/08/2011 07:11, Lex Trotman a écrit :
On 22 August 2011 22:57, Colomban Wendling lists.ban@herbesfolles.org wrote:
[...] However it'd be easy to add "auto-detect" to indentation type and width selection menus -- though indentation width detection doesn't work for tabs-based indentation for now.
The autodetect entry is to provide a way to return it to autodected values after I mucked it up by fiddling and can't remember where I started :-)
So, to summarize a little:
Add per-filetype indentation width and type;
When setting the filetype for the first time (file open and unpon
initial manual filetype selection), use, in order: a) auto-detect (if preferred)
or not new file (can't detect a non-existent file :-)
b) filetype setting c) user global settings d) system global settings
- when changing the filetype for the second (or more) time, don't touch
indentation settings.
Sounds good?
Yep.
I've finally committed this to SVN, see r5902 and r5904. Still possible to comment if you have any further suggestions :)
Cheers, Colomban
[...]
I've finally committed this to SVN, see r5902 and r5904. Still possible to comment if you have any further suggestions :)
Who, me, comment? Never :-)
I can't try it due to lack of access to dev machine and looks like it will be another week before I get back to it, but looks ok from cursory inspection.
Cheers Lex