This repository has been archived by the owner on Sep 6, 2021. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 7.6k
Added smart indent and auto close tags as preferences #6888
Merged
Merged
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a little uncertain about having this be an object pref vs. a boolean or some combination of other prefs. It makes this a bit more complex to use. That said, I looked at closetag.js and seeing how these options work, I can imagine people reasonably wanting to tweak these different values.
Aside: I wonder why we override indentTags?
One possibility is to start with just a
closeTags
boolean that's either all on (the definition above) or all off and then define other (less likely to be used) preferences to customize the behavior further. I could imagine indentTags being a separate pref, for example.What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Initially I just wanted to add
whenOpening
andwhenClosing
as boolean prefs, since those are the useful ones. People would like to disable one but not both and other disable both. But I wasn't sure how it would work when updating the preference in CodeMirror. I guess if I go that way it would require a special case to handle it.So I am ok in using 2 boolean pref and a special case to handle the on change event.
We overwrite
indentTags
because if we don't then when you have<div|
and write a>
it auto-completes and indents the tag becoming:Which is really annoying, but some people might like it. So you can define for which tags this behavior is ok.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dangoor Want me to do that instead of using the object?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@TomMalbran Sorry I didn't get back to this earlier. Given your description, I'd say we can just leave it as an object. We may make some changes when we do the UI, but I don't think it's worth adding a bunch of special case code at this point.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No problem. Just trying to make sure to get this into Release 37 :)
I guess the UI might be a problem. A simple UI where we just edit a JavaScript Object with lots of comments and all the possible preference would just work fine. A more complex ones with inputs should have some sort of API where each module can add new input lines for each preference to add to the UI, and that API could handle inputs as objects.