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

Fix tabs in xi term (mostly) #85

Merged
merged 2 commits into from Oct 27, 2018

Conversation

ktomsic
Copy link
Contributor

@ktomsic ktomsic commented Oct 26, 2018

Note: this PR depends on work in xi-frontend/xrl#18

Xi-term doesn't position the cursor properly when its tab width differs from the terminal tab width (#80). We can work around this in two ways:
(1) Allow the translate_tabs_to_spaces option (when enabled) to avoid the problem entirely (translate tabs into spaces = no tabs = no problems) and
(2) Allow the user to change the tab width in their preferences.xiconfig file. This still requires a correct setting, but it at least gives the user the option to change xi-term to work with their terminal rather than the other way around.

Currently, we require the user to configure their terminal tab width to
four columns wide. One way we can get around this is to expand tabs into
spaces. Xi-core can do this for us -- there's actually a
'translate_tabs_to_spaces' config option, which is enabled by default.
In order for it to work, though, we have to indicate to xi-core when we
receive a tab via the "insert_tab" BufferEvent, so this changeset does
so.

For consistency, we also send "insert_newline" on newlines.
If the terminal tab settings don't match our concept of tab locations,
funny things can happen when we try to position the cursor on a line
with tabs.

We can work around this a couple of ways:

(1) We can expand tabs into spaces. Xi-core does this by default for
most file types (Makefiles are a notable exception). Spaces obviously
aren't affected by the terminal tabstop settings, so we avoid the
problem entirely.

(2) Alternatively, we can allow the user to specify the tab size. Rather
than assuming tabs are 4 columns wide, we'll assume that the user's
preferences.xiconfig and terminal settings match up. Views will now
store a tab_size setting from the last ConfigChanged event they
received, and this setting will be used to position the cursor.

Note that we'll still have problems if the terminal and xi-term tab
settings don't agree, but hopefully this will reduce their occurrence
somewhat.
Copy link
Member

@wendivoid wendivoid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This look's really good thank you! and i'm sorry it took a whole day to review this something came up and i couldn't finish until now

@wendivoid wendivoid merged commit 3ec107a into xi-frontend:master Oct 27, 2018
@ktomsic
Copy link
Contributor Author

ktomsic commented Oct 30, 2018

No worries. Thanks for taking the time to review + merge it!

@ktomsic ktomsic deleted the fix-tabs-in-xi-term branch October 30, 2018 01:16
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.

None yet

2 participants