Skip to content
This repository has been archived by the owner on Jun 25, 2020. It is now read-only.

Indentation cleanup, added smart_brackets and smart_insertions, and more #284

Merged
merged 22 commits into from Sep 15, 2016

Conversation

eidheim
Copy link
Member

@eidheim eidheim commented Sep 5, 2016

@d3rrial, @zalox and others: I need testing of this PR before merge:

  • Added move one ) instead of insert, which is very nice after autocomplete (Autogenerated closing brackets #281)
  • Major cleanup of the indenting code, it's actually much faster compared to current master, and additionally more readable (I hope).
  • smart_insertions
  • Various other improvements

@eidheim
Copy link
Member Author

eidheim commented Sep 5, 2016

And the indentation is improved as well. For instance it should now correctly indent:

some_function(true, 42,
              [] {//enter here

, that will lead to:

some_function(true, 42,
              [] {
  //cursor here
}

@insunaa
Copy link
Contributor

insunaa commented Sep 5, 2016

void asdf(std::string& superduperlongvariablenamewtf, std::string& andanotheronewhowouldbelievethis,
          std::string& thisiswaytoomuch){

          }

Is the indent here as intended? Looks strange to me.

Other than that the PR looks good to me.

@eidheim
Copy link
Member Author

eidheim commented Sep 5, 2016

@d3rrial Thank you, that is definitely a bug. I'll fix it Tomorrow.

@eidheim
Copy link
Member Author

eidheim commented Sep 6, 2016

I fixed the bug @d3rrial mentioned. Also improved indentation when writing constructor implementations on multiple lines. Let me know if there are other issues:)

@eidheim
Copy link
Member Author

eidheim commented Sep 6, 2016

See also the last commit. You can now avoid using arrow keys when for instance writing:

auto c=std::make_shared<int>(10);

bool find_open_curly_bracket_backward(Gtk::TextIter iter, Gtk::TextIter &found_iter);
bool find_close_curly_bracket_forward(Gtk::TextIter iter, Gtk::TextIter &found_iter);
long open_close_paranthesis_count(Gtk::TextIter iter);
long open_close_bracket_count(Gtk::TextIter iter, unsigned int left_bracket, unsigned int right_bracket);
Copy link
Contributor

Choose a reason for hiding this comment

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

unsigned int? Are you sure? Wouldn't that mean the '(' has to be cast from char? Or does Clang handle single quoted symbols differently?

Copy link
Member Author

Choose a reason for hiding this comment

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

*iter returns unsigned int since it is a UTF-8 character. So I do the cast automatically in the function parameters. At least the tests passed:)

Copy link
Contributor

Choose a reason for hiding this comment

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

Well it wasn't an error, at most I would have expected the compiler to throw a warning. As long as it works, it's all good :)

@insunaa
Copy link
Contributor

insunaa commented Sep 6, 2016

auto c=std::make_shared<int>(10);
Works well, but not flawlessly (this might be out of scope for this PR, tho):

when I type auto c=std::make_sha I get an autocomplete suggestion for

auto c=std::make_shared<typename _Tp>(_Args &&__args...)

where my cursor sits in the angled bracketes with typename_Tp being highlighted (so it's overwritten when I start typing). To get past that initial > after the typename_Tp I still have to use the arrow key to skip it, tho. Other than that, it works perfectly.

@eidheim
Copy link
Member Author

eidheim commented Sep 6, 2016

In the case of auto c=std::make_shared<typename _Tp>(_Args &&__args...) and you replace typename _Tp you should get moved when entering > just before the >. You should be able to enter both template argument and function argument without using the arrow keys after using autocompletion on std::make_shared.

@insunaa
Copy link
Contributor

insunaa commented Sep 6, 2016

This doesn't work for me. Could it be because of different keyboard layouts / keyval differences?

@eidheim
Copy link
Member Author

eidheim commented Sep 6, 2016

Yeah, tested on Linux. Have to find the right key that works (GDK_KEY_greater works on OS X).

@insunaa
Copy link
Contributor

insunaa commented Sep 6, 2016

I just tested and GDK_Key_greater is keycode 62, but 62 is also they keyval for > for me, so that's probably not the cause...

@eidheim
Copy link
Member Author

eidheim commented Sep 6, 2016

The problem is searching for < and > keys it seems. I'll see if I can fix it later.

@insunaa
Copy link
Contributor

insunaa commented Sep 6, 2016

Well, < and > are also comparison operators after all. In the case where it's used as a templating mechanism, I think it's enough to stop as soon as any other type of bracket is found. I can't think of a situation where <...)...> or <...]...> or <...}...> / <...(...> or <...[...> or <...{...>would be valid constructs.

auto a=crazytemplate<std::string, void*, int,
                     double, size_t>(...);

This has to remain valid, tho...

Edit: Are typeof() functions used in templating? Because if they are, then this is probably a moot point.

…ion arguments of autocompleted templated functions. Also selects the text in the function arguments after pressing '('.
@insunaa
Copy link
Contributor

insunaa commented Sep 6, 2016

Looks good to me, even works with nested templates. Haven't done any edge-case tests for false positives yet, but I haven't had any false negatives. 👍

@eidheim
Copy link
Member Author

eidheim commented Sep 6, 2016

Thank you, good to hear. At least there is some progress:)

@eidheim
Copy link
Member Author

eidheim commented Sep 6, 2016

I made the rules for the movements very strict so that they will not happen unless you want them to, hopefully.

@insunaa
Copy link
Contributor

insunaa commented Sep 6, 2016

While I personally love this feature / these movements and will/would use them all the time, for some people this might be an inconvenience (I can't fathom how, because they behave just like regular, solid brackets in any case other than when you type in a bracket which is already there) so I think it might be best if this was put into a setting in the config file.

@eidheim
Copy link
Member Author

eidheim commented Sep 6, 2016

I'm thinking that too. I'll use it myself, but we should maybe have it disabled by default. What do you think @zalox ?

@zalox
Copy link
Member

zalox commented Sep 6, 2016

See #281 (comment).

I see no reason to not enable these features by default. It would effectively improve the coding experience for new users. People like me, that don't like magic features, know how to disable them and keep quiet as long as there is an option to do so.

@eidheim
Copy link
Member Author

eidheim commented Sep 7, 2016

So what should we call this bracket feature in the preferences? smart_brackets? I'll add some comment as well (smart_brackets_comment).

@eidheim
Copy link
Member Author

eidheim commented Sep 7, 2016

Not completely sure about the preference item name smart_brackets, but at least the functionality is there.

@eidheim eidheim force-pushed the indentation_cleanup branch 2 times, most recently from 564c868 to 55650ac Compare September 10, 2016 08:35
@eidheim
Copy link
Member Author

eidheim commented Sep 10, 2016

Fixed the spellcheck issue on undo/redo/pasting.

@eidheim eidheim force-pushed the indentation_cleanup branch 6 times, most recently from fff7bfc to 3b8ec87 Compare September 10, 2016 16:36
@zalox
Copy link
Member

zalox commented Sep 11, 2016

Shouldn't the { also be smart_inserted?

@eidheim
Copy link
Member Author

eidheim commented Sep 11, 2016

I tried doing smart insertion of {}, but found that too often that was not what I wanted. Additionally, we already have smart indentation and insertion of } after pressing enter after {, and this leads to far less false positives.

@eidheim eidheim changed the title Indentation cleanup, added smart_brackets and smart_insertions Indentation cleanup, added smart_brackets and smart_insertions, and more Sep 12, 2016
@eidheim
Copy link
Member Author

eidheim commented Sep 12, 2016

@zalox This PR should be getting ready for merge. I added some more improvements that should be pretty safe. The biggest change is the indentation cleanup, and that seems to work fine.

@eidheim
Copy link
Member Author

eidheim commented Sep 13, 2016

@zalox this PR should now be ready for merge!:)

@eidheim
Copy link
Member Author

eidheim commented Sep 15, 2016

I'll merge this now as this PR is stable and contains numerous improvements.

@eidheim eidheim merged commit 43eaad0 into cppit:master Sep 15, 2016
@eidheim eidheim deleted the indentation_cleanup branch September 15, 2016 07:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants