Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[feature request] expand tabs option based on path #1222

Closed
unxed opened this issue Jan 30, 2022 · 47 comments
Closed

[feature request] expand tabs option based on path #1222

unxed opened this issue Jan 30, 2022 · 47 comments

Comments

@unxed
Copy link
Contributor

unxed commented Jan 30, 2022

putty's sources use spaces. far2l uses tabs. Continuously switching between modes produces mixed files, which is mess. It would be great to have an option to configure "expand tabs" editor option differently for different paths.

@elfmz
Copy link
Owner

elfmz commented Jan 30, 2022

or may be even autodetect..

@unxed
Copy link
Contributor Author

unxed commented Feb 19, 2022

default editor's charset by path is also appreciated

@unxed
Copy link
Contributor Author

unxed commented Mar 3, 2023

Please implement if possible. Still hurts.

@elfmz
Copy link
Owner

elfmz commented Mar 5, 2023

See branch edit-live-expandtab
There was imlemented changing tabs/spaces by F5 in editor and saving tabs expansion per-file in history.
Planned improvement is Shift+F5 to allow per-file spaces count modification on the fly and may be increasing history elements count from 512 to 4096 or so.. Later will cause old histories to be lost BTW.

@unxed
Copy link
Contributor Author

unxed commented Mar 5, 2023

Great! Will look within a half of an hour. Maybe it is possible to detect indentation style for the first time opened files also?

@unxed
Copy link
Contributor Author

unxed commented Mar 5, 2023

Looks working!

@unxed
Copy link
Contributor Author

unxed commented Mar 5, 2023

With this simple addition I've got just that I wanted :)

diff --git a/far2l/src/fileedit.cpp b/far2l/src/fileedit.cpp
index 83373f59..c7d21392 100644
--- a/far2l/src/fileedit.cpp
+++ b/far2l/src/fileedit.cpp
@@ -1530,6 +1530,8 @@ int FileEditor::LoadFile(const wchar_t *Name,int &UserBreak)
 	EditFile.GetSize(FileSize);
 	DWORD StartTime=WINPORT(GetTickCount)();
 
+	int ConvertTabsMode = m_editor->GetConvertTabs();
+
 	while ((GetCode=GetStr.GetString(&Str, m_codepage, StrLength)))
 	{
 		if (GetCode == -1)
@@ -1589,6 +1591,14 @@ int FileEditor::LoadFile(const wchar_t *Name,int &UserBreak)
 			LastLineCR=1;
 		}
 
+		// detect indentation mode by the last string of file
+		// what starts with tab or two spaces
+		if (Str[0] == '\t') {
+			ConvertTabsMode = EXPAND_NOTABS;
+		} else if ((Str[0] == ' ') && (Str[1] == ' ')) {
+			ConvertTabsMode = EXPAND_NEWTABS;
+		}
+
 		if (!m_editor->InsertString(Str, StrLength))
 		{
 			EditFile.Close();
@@ -1596,6 +1606,33 @@ int FileEditor::LoadFile(const wchar_t *Name,int &UserBreak)
 		}
 	}
 
+	// tab mode auto detection
+	if (m_editor->GetConvertTabs() != EXPAND_ALLTABS) {
+		m_editor->SetConvertTabs(ConvertTabsMode);
+	}
+
+	// trying to apply .editorconfig setting
+	// sudo apt-get install editorconfig
+	wchar_t command[4137];
+	std::wstring NameStr = Name;
+	QuoteCmdArg(NameStr);
+	swprintf(command, 4136, L"editorconfig %ls | grep indent_style=space", NameStr.c_str());
+	if (!system(Wide2MB(command).c_str())) {
+		m_editor->SetConvertTabs(EXPAND_NEWTABS);
+	}
+	swprintf(command, 4136, L"editorconfig %ls | grep indent_style=tab", NameStr.c_str());
+	if (!system(Wide2MB(command).c_str())) {
+		m_editor->SetConvertTabs(EXPAND_NOTABS);
+	}
+	swprintf(command, 4136, L"editorconfig %ls | grep tab_width=2", NameStr.c_str());
+	if (!system(Wide2MB(command).c_str())) {
+	    m_editor->SetTabSize(2);
+	}
+	swprintf(command, 4136, L"editorconfig %ls | grep tab_width=4", NameStr.c_str());
+	if (!system(Wide2MB(command).c_str())) {
+	    m_editor->SetTabSize(4);
+	}
+
 	BadConversion = !GetStr.IsConversionValid();
 	if (BadConversion)
 	{

@akruphi
Copy link
Contributor

akruphi commented Mar 5, 2023

See branch edit-live-expandtab There was imlemented changing tabs/spaces by F5 in editor and saving tabs expansion per-file in history. Planned improvement is Shift+F5 to allow per-file spaces count modification on the fly and may be increasing history elements count from 512 to 4096 or so.. Later will cause old histories to be lost BTW.

Extra fine! Very useful features! But it required clear simple info about current mode. In top status line in Editor some info will be useful:

  • number of current tab spaces
  • TAB as \t or expand to spaces
  • sign about on/off tab autodetection

For example may be it write as:
AT8 - autodetect tab as \t 8 symbols
S4 - manually tab as 4 spaces
etc

@akruphi
Copy link
Contributor

akruphi commented Mar 5, 2023

May be useful also add in Editor switch to show spaces, tabs and new lines as special symbols (may be more gray than standard text color). For example: each space as · U+00B7, tab as U+21E5 or U+21B9 , new lines as U+21B5 , carriage return as ??? (not now idea).

@elfmz
Copy link
Owner

elfmz commented Mar 5, 2023

There is already option Show white space in F9/Options/Editor settings

@akruphi
Copy link
Contributor

akruphi commented Mar 5, 2023

There is already option Show white space in F9/Options/Editor settings

Ooops! Fine! Please add keyboard shortcut to on/off show white space in Editor on-fly. May be by F3 or F9, which now free.

@elfmz
Copy link
Owner

elfmz commented Mar 5, 2023

Whitespaces toggling added on Ctrl+F5. This however not saved per file, should it be? IMHO not, but can reconsider..

@elfmz
Copy link
Owner

elfmz commented Mar 5, 2023

Merged to master, regarding autodetection - may be will do it as an extra option, i just afraid that it will be too much unpredictable

@unxed
Copy link
Contributor Author

unxed commented Mar 5, 2023

Merged to master, regarding autodetection - may be will do it as an extra option, i just afraid that it will be too much unpredictable

Let user decide: it may be enabled as fourth "expand tabs" option: "expand new tabs if no tabs present in file".

@unxed
Copy link
Contributor Author

unxed commented Mar 5, 2023

There are several examples of tab detection, may be it worth looking at them to select best logic:
https://unix.stackexchange.com/a/478448

@elfmz
Copy link
Owner

elfmz commented Mar 5, 2023

Its still heuristics, no matter how its done. Also, i think if implementing this then need also check 'sibling' files in same in case new file being created or in case algorithm assumed unreliable decision.

@unxed
Copy link
Contributor Author

unxed commented Mar 5, 2023

check 'sibling' files

Sounds good

@unxed
Copy link
Contributor Author

unxed commented Mar 5, 2023

When you press F5 in tab mode, all tabs in the file are replaced with spaces. I do not think it's a good idea. It would be better to switch to tab mode 2, rather than tab mode 1, so that the switch only affects newly entered characters, not existing ones.

@elfmz
Copy link
Owner

elfmz commented Mar 5, 2023

may be its better to save tabs state only if it was modified explicitly (by F5)? and keep default if it wasnt?

@unxed
Copy link
Contributor Author

unxed commented Mar 5, 2023

may be its better to save tabs state only if it was modified explicitly (by F5)? and keep default if it wasnt?

agree

@akruphi
Copy link
Contributor

akruphi commented Mar 6, 2023

Whitespaces toggling added on Ctrl+F5. This however not saved per file, should it be? IMHO not, but can reconsider..

Thank you. Very useful features, which usually need in moment to verify real formatting (now in far2l show contrast bright white-spaces with colors equal to text not comfort during regular editing). I think it not need to save individually per file.

For me toggle white-space more often action then change tab spaces and may be if it possible switch toggling to 1-key (may be from Clrt+F5 to F3), but here I not sure.

@elfmz
Copy link
Owner

elfmz commented Mar 6, 2023

first i suspect that other still free keys can be used by plugins or by some future useful functionality so i don't like reserving all F-s right now, second all that whitespaces/tabs-to-space convertions are similar and so looks better to be on same F with different mods...

@akruphi
Copy link
Contributor

akruphi commented Mar 6, 2023

first i suspect that other still free keys can be used by plugins or by some future useful functionality so i don't like reserving all F-s right now, second all that whitespaces/tabs-to-space convertions are similar and so looks better to be on same F with different mods...

I agree. Thank you for efforts & fine far2l. May be change F5 to toggle white-space and Ctrl+F5 to toggle space/tab mode? What opinion @unxed about what action more often change tabs or show white-space?

@unxed
Copy link
Contributor Author

unxed commented Mar 6, 2023

No opinion. Should use new features for a while before making conclusions. Also, my UX is specific: I use tabs auto detection patch, so do not need to switch tab mode manually often. With all it's simplicity, for all source code files I've tested it with it gave me no failed detections. It uses the last line of the file what starts with a tab character or two spaces to determine the type of indentation. There is sometimes mixed formatting at the beginning of source code files due to different headers and comments, using end of file for auto detection works better because the end of any source code file usually looks something like this:

    }
}

@unxed
Copy link
Contributor Author

unxed commented Mar 6, 2023

As for determining the tab mode for new files, I could not come up with a sufficiently reliable algorithm. What if it's the first file in the folder? What to do with files with different extensions, like .c and .h? Probably the only reliable solution would be to use per-folder configuration files .far2l with contents like

[Editor]
ExpandTabs=2

If no such file is found in the current folder, far2l should look in parent folders through directory tree up to the root folder. If still no such file found, auto detection should be used, and for the new files global editor setting should be used. Both features, per-folder configuration and tab mode detection should be implemented as optional, being enabled in global configuration like:

[x] Enable per-folder .far2l config files support // enabled by default
[ ] Enable tab mode auto detection // disabled by default

With both whose checkboxes disabled far2l should behave just like it behaves now.

Also auto detection may be improved by asking user explicitly during opening file:
"Tab indentation detected in file, but expanding tabs to spaces is enabled. Do you want to switch expanding tabs to spaces off for this file to avoid mixed indentation?"

@elfmz
Copy link
Owner

elfmz commented Mar 6, 2023

maybe this?
https://editorconfig.org/

@unxed
Copy link
Contributor Author

unxed commented Mar 6, 2023

https://editorconfig.org/

Looks good!

@unxed
Copy link
Contributor Author

unxed commented Mar 6, 2023

Wow, libeditorconfig is even available in Ubuntu/Fedora/brew repos!

~$ sudo apt-get install editorconfig
Чтение списков пакетов… Готово
Построение дерева зависимостей… Готово
Чтение информации о состоянии… Готово
Будут установлены следующие дополнительные пакеты:
  libeditorconfig0
Следующие НОВЫЕ пакеты будут установлены:
  editorconfig libeditorconfig0

@elfmz
Copy link
Owner

elfmz commented Mar 6, 2023

format doesnt look as something much more than usual ini file with few simple extensions so better to avoid extra dependency

@unxed
Copy link
Contributor Author

unxed commented Mar 6, 2023

It turns out that several projects I work with already use this format. It's great if far2l starts supporting it out of the box!

@akruphi
Copy link
Contributor

akruphi commented Mar 7, 2023

Because now active working with the editor may be also implemented in it quick switching F8 between not only OEM<->ANSI, but also UTF8<->OEM<->ANSI (it will closed #1179). May be even realize for user customization codepages for F8.

@elfmz
Copy link
Owner

elfmz commented Mar 8, 2023

i thought there can be no conflicting rules, now i see that it can and must be ordered, so need some corrections.. Regarding ** - its also not yet supported

@unxed
Copy link
Contributor Author

unxed commented Mar 8, 2023

Also I have some problems with my .editorconfig files for far2l, checked on vanilla editorconfig tool, need some fixes. Please apply #1530.

@unxed
Copy link
Contributor Author

unxed commented Mar 8, 2023

Still problems :) See calc/.editorconfig, then try to edit calc/src/shared/sgml/tools.cpp.
Far opens it as T4, but command line editorconfig tool reports another settings:

charset=utf-8
end_of_line=lf
indent_style=space
indent_size=2
tab_width=2

@unxed
Copy link
Contributor Author

unxed commented Mar 8, 2023

After some experiments I found that far2l treats sections names with relative paths as if we are in folder of file edited, even if applied section is from file located in one of parent folders.

@elfmz
Copy link
Owner

elfmz commented Mar 8, 2023

There are 2 hard problems in computer science: cache invalidation, naming things, and off-by-1 errors

@unxed
Copy link
Contributor Author

unxed commented Mar 8, 2023

Thanks!

In the meantime, I made some attempt to get rid of the mixed indentation in the sources, take a look: #1532 (build error fixed!).

By the way, how about running sources (excluding ones that are often upstream-synced) through astyle utility for consistent indentation style and code beautifulness? I experimented with it, works great. And for the cmake there is cmake-format that does the same job.

UPD: Verified all far2l's .editorconfig files, now everything seem to work fine, great!

@elfmz
Copy link
Owner

elfmz commented Mar 9, 2023

I suddenly found that Alt+Shift+F9 in editor opens 'local' editor configuration. However it doesnt work for some reason per se, but works if press Win+Shift+F9.
So two questions arose then:

  1. Why so tricky combination for such useful dialog, why not simple F9 that is free in Editor?
  2. Given if reassign this to F9 - is recently added F5 and related really needed?

@elfmz
Copy link
Owner

elfmz commented Mar 9, 2023

Same for Viewer BTW

@unxed
Copy link
Contributor Author

unxed commented Mar 9, 2023

Win+Shift+F9

Not working for me :(

@unxed
Copy link
Contributor Author

unxed commented Mar 9, 2023

In tty-xi Shift-Alt-F9 works fine. Cool thing! Still, tab mode hot keys were very useful during mixed indentation removal that I did in last few days.

@elfmz
Copy link
Owner

elfmz commented Mar 9, 2023

I have language switcher at Alt+Shift, may be this prevents Alt+Shift+F9. So i will reassign it to F9, as Alt+Shift+F9 not obvious and as found - not working sometimes..

@unxed unxed closed this as completed Mar 11, 2023
@unxed
Copy link
Contributor Author

unxed commented Mar 11, 2023

btw, what is preferred way of indenting multi line if statements when using tabs? With space indentation I can write something like:

if (
    (a == b) &&
    (c == d)
   )
{
    doStuff();
}

but with tabs indentation it becomes less beautiful:

if (
	(a == b) &&
	(c == d)
	)
{
    doStuff();
}

Or maybe such alignments are not welcomed at all when indenting is done via tabs?

@unxed
Copy link
Contributor Author

unxed commented Mar 13, 2023

Btw, github announces .editorconfig support, but diffs are still viewed with 8-space-tabs. What am I doing wrong?

@unxed
Copy link
Contributor Author

unxed commented Mar 13, 2023

strange thing. both files should share the same .editorconfig settings

@elfmz
Copy link
Owner

elfmz commented Mar 14, 2023

May be it takes .editorconfig from 'old' revision?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants