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

add automatic tabsize detection #114

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jkittner
Copy link
Contributor

Hi,
I tried implementing an automatic tabsize detection when opening a file.

  • I added a few test cases for the tabsize detection
  • I had to modify the test_dedent_selection test, because the tab size was detected as 2 tabs. I think it actually wants to test that lines are properly dedented, even if the indentation is smaller than the currently set tab_size right? I think the test still does this now?

I am not sure if this detection will fit all different kinds of files or if the file type has to be taken into consideration...

@asottile
Copy link
Owner

ah hmm so the idea that I had for this was instead to mark the default tab settings in each language file: asottile/babi-grammars#27 (and probably also solve this at the same time: asottile/babi-grammars#26) and then use that when opening files

this of course wouldn't handle people breaking convention (google using 2-space python for example) but would provide a decent baseline

later when configuration is implemented, an override per language could be done

this better solves the "I create an empty file" problem since auto-detection doesn't really work there

what are your thoughts on this?

@jkittner
Copy link
Contributor Author

this of course wouldn't handle people breaking convention (google using 2-space python for example) but would provide a decent baseline

you mean using hard tabs which are expected to expanded to length two?

later when configuration is implemented, an override per language could be done

year this would help -- maybe my proposed implementation might still be able to act as a fallback for unknown languages in this case?

this better solves the "I create an empty file" problem since auto-detection doesn't really work there

yes, I agree for an empty file it would still just use the current default of 4 spaces...

@asottile
Copy link
Owner

this of course wouldn't handle people breaking convention (google using 2-space python for example) but would provide a decent baseline

you mean using hard tabs which are expected to expanded to length two?

I think it would be something like "indent_string": "\t" or "indent_string": " " or "indent_string": " " but admittedly I haven't put much thought into it

later when configuration is implemented, an override per language could be done

year this would help -- maybe my proposed implementation might still be able to act as a fallback for unknown languages in this case?

ah yeah that's a good point so it would still be useful there

@asottile
Copy link
Owner

though hmm, maybe "unknown" should always default to 4-spaces which puts incentive to add language support

@jkittner
Copy link
Contributor Author

yes I think your language specific approach is definitely the proper way to go and will be right in more cases. The only case where my solution would be helpful with added language support would be opening an existing file with an unknown language -- which is obviously a rare case and defaulting to 4 spaces would probably be right a similiar amount.

I don't think I am able to implement the language specific part though since it will include the proper grammar parsing and I have no idea about this.
I wonder how vim, vscode and other IDEs/editos do this? I definitely had vscode choosing the wrong one for be before, but I haven't looked into the why and just switched manually..

@asottile
Copy link
Owner

I think vs code chooses the indent size per language, but stored external to the grammars -- I haven't found any code for this though

my thought was to augment the grammars with our own information 🤷

@jkittner
Copy link
Contributor Author

I had a bit of a look into vscode but did not actually find a complete answer. For me it looks like they guess the indentation from the file also without taking the language into account I found this test and the editor default settings as well as a class called DetectIndentation which I don't really understand...

Seems like this is a much more complex problem than I initially though. I might take a deeper look into this later and come up with a new approach, but since I am not familiar with this at all atm, it might take quite some time which I unfortunatelly don't have during my semester.
So feel free to close this for now. Sorry for the noise, but I definitely got a better understanding what is actually needed now thank you! :)

@asottile
Copy link
Owner

I think it's fine to leave this open while we investigate! Hopefully we can figure this out together

@jkittner
Copy link
Contributor Author

just to add one more idea for now:
vscode allows language specific configuration in an editor config file like this

  "[Python]": {
    "editor.detectIndentation": false,
    "editor.tabSize": 4,
    "editor.insertSpaces": true
  }

so we could add a per language default (for all languages currently supported with babi-grammars) in a config file, but also allow a user config file in .config/babi which overwrites the defaults in case you want to use 2 space indentation in python etc.
so when babi opens the file and parsess the fileextension we could take the information from the user config file if the language is in the keys or from the default config(file or dict). The obvious downside is, that the file is opened each time babi starts which might result in a slower start ?

I am not sure if we can get any other information other than the file types from the grammar?

@asottile
Copy link
Owner

yeah I want to have some sort of user configuration eventually, I don't think we need to implement it yet though:

later when configuration is implemented, an override per language could be done

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

Successfully merging this pull request may close these issues.

2 participants