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

[frontend] Fixing indentation problems + indentation preferences #995

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

AlexandreDoneux
Copy link
Contributor

Tab indentation is not usable in python because of Python inability to mix spaces and tabs for indentation (see issue #323).

This PR fixes this by giving the user the choice of the type of indentation it desires (2, 3, 4 spaces or 4 space tabs). This applies to the tab key and auto-indent.

…ing ancient DB update when connecting user

Not setting the code indentation in DB if the user has not changed it from default allows the gain of space in the database. A database update was present in the user connection because the idea in the past was to have the preferences changed in session and in DB through the same method. This was aaplied for the username change but was not done later for other preferences. A DB update was added later in profile.py despite beeing already present in connect_user().
@AlexandreDoneux AlexandreDoneux marked this pull request as ready for review February 27, 2024 14:56
@AlexandreDoneux AlexandreDoneux changed the base branch from master to v0.9.0.rc0 March 21, 2024 13:43
@AlexandreDoneux AlexandreDoneux changed the base branch from v0.9.0.rc0 to master March 21, 2024 13:44
Copy link
Member

@anthonygego anthonygego left a comment

Choose a reason for hiding this comment

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

According to the CodeMirror code, we should not need to use searchCursor. Besides, the current implementation will act incorrectly in a large number of cases.

There may be an issue with the current copy in our repo.

inginious/frontend/app.py Outdated Show resolved Hide resolved
inginious/frontend/app.py Outdated Show resolved Hide resolved
inginious/frontend/installer.py Outdated Show resolved Hide resolved
inginious/frontend/pages/preferences/profile.py Outdated Show resolved Hide resolved
inginious/frontend/pages/register.py Show resolved Hide resolved
}

if (user_indentation_type["text"] == "tabs") {
keyMappings["Tab"] = function(cm) { cm.execCommand("insertSoftTab"); let text = cm.getSearchCursor(' '); while(text.find()){text.replace("\t");}; };
Copy link
Member

Choose a reason for hiding this comment

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

Did you try insertTab instead of insertSoftTab here ?

See https://github.com/codemirror/codemirror5/blob/master/src/edit/commands.js#L96

It looks like the current code would replace all the 4-space-length blocks, even the ones put on purposes, like the one in this line of code.

Copy link
Contributor Author

@AlexandreDoneux AlexandreDoneux Apr 3, 2024

Choose a reason for hiding this comment

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

All the 4-space-length blocks will indeed be replaced by tabs if it is the indentation method the user has selected. It is perfectly intended because of Python not handling mixed indentation of spaces and tabs.

It has nothing to do with insertTab and insertSoftTab. The difference between those two is that the first simply adds a "\t" and the second completes an unfinished indentation (based on the indentUnit) and replaces it with a "\t".

Correction :
insertTab inserts a "\t" character at the place of the cursor to match the indentation given by tabSize.
insertSoftTab inserts a certain amount of spaces to match the indentation given by tabSize.

The problem with insertTab is it does not replace the already existing spaces so there is still a mix of spaces and tabs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have found a better way to avoid replacing all quadruple spaces in the code with tabs. I use insertTab, identLess and insertTab again. This will have the same effect of completing a tab and replacing 4-space blocks with tabs but only the ones used for indentation.

What happens is we complete the tab with insertTab, delete it with indentLess and recreate it in full with the second insertTab. All the previous indentation spaces are replaced by tabs because of the way indentLess works and because we use indentWithTabs=true.

inginious/frontend/app.py Outdated Show resolved Hide resolved
inginious/frontend/user_manager.py Show resolved Hide resolved
Plus adding back default values in DB. They are many cases where we don't handle if data is not present in the DB.
Make text translatable, unique indent value for indentUnit and tabSize, removing from globals
Now autocompletes tab indentations and replacing 4-space blocks with tabs, but only those used for indentation.
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