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

[Clojure] syntax rework #1259

Merged
merged 24 commits into from Nov 28, 2018

Conversation

Projects
None yet
6 participants
@mitranim
Contributor

mitranim commented Oct 24, 2017

This PR is a complete rework of the Clojure syntax.

Improvements:

  • The syntax structure is modeled after the AST and the Clojure reader. Should
    handle any formatting, regardless of whitespace and indentation quirks.

  • No unnecessary special cases. Simple and consistent rules. User-defined
    syntax should look and feel the same as standard syntax.

  • All syntactic forms are correctly scoped in file root

  • Much better at handling literals such as numbers, keywords, etc

  • Works for custom defs, e.g. defroute

  • Correctly declares definterface, defprotocol, deftype, defrecord types

  • Correctly highlights methods in definterface, defprotocol, deftype, defrecord

  • Correctly declares functions defined by defprotocol

  • Correctly declares names in the presence of metadata, whitespace, comments

  • Correctly handles defmulti and defmethod

  • Doesn't declare declare (noise)

  • Highlights mismatched brackets, helping with balancing

  • Consistent highlighting of function calls

  • ≈ 4 times smaller

  • ≈ 6 times faster

Doesn't detect function parameters and local bindings. I'm not sold on supporting 8+ special assignment forms, each with their own quirks, in a language with macros. It immediately breaks when a user introduces their own variation (example of a slightly different let I use in practice). Suggestions are welcome.

Would appreciate review.

Edit: while the PR is incubating, the syntax is available as standalone at https://github.com/Mitranim/sublime-clojure

@jrappen

This comment has been minimized.

Contributor

jrappen commented Oct 24, 2017

1)

⚠️ There are no tests in your test files...?

Compare the test for Clojure:

Also, your package structure:

  • One test file:
    /Clojure
        Clojure.sublime-syntax
        syntax_test_clojure.clj
    
  • several test files:
    /Clojure
        Clojure.sublime-syntax
        /tests
            syntax_test_clojure.clj
            syntax_test_clojure_benchmark.clj
            ...
    

2)

Using whitespace for a vertical separation of blocks is nice to the eyes (at least mine...). For the syntax files as well as the test(s).

Compare the Python subfolder:

./Python

also great, LaTeX:

./LaTeX

@rwols

rwols suggested changes Oct 24, 2017 edited

I have no clue about Clojure, but here are some suggestions about the .sublime-syntax file itself that you could take in account. It'd be nice if we had another Clojure expert.

|\d+(?:\.\d+)?(?:[Ee][-+]?\d+)?M? # decimal with scientific notation
)
(?=[{{non_number_chars}}])
reader_macro_scope: constant.other.symbol.reader-macro.clojure

This comment has been minimized.

@rwols

rwols Oct 24, 2017

Contributor

You cannot assign variables to scopes. Remove this variable.

pop: true
bindings_form:
- include: match_expr
match_expr:

This comment has been minimized.

@rwols

rwols Oct 24, 2017

Contributor

Style: use-dashes-for-contexts, and use_underscores_for_variables.

- include: match_noise
- match: '[)\]}]'
scope: invalid.illegal.clojure
- match: '\('

This comment has been minimized.

@rwols

rwols Oct 24, 2017

Contributor

You can omit the redundant single quotes around this regex, and various other regexes. Download PackageDev and enable the "Sublime Syntax" syntax from that package to better see what is possible and what is not possible within regexes. I highly recommend that package.

pop_fn_list_tail:
- include: match_constant_set_normal_list_tail
- match: '{{symbol}}'
scope: entity.name.fn.clojure

This comment has been minimized.

@rwols

rwols Oct 24, 2017

Contributor

entity.name.function?

;; ^^ punctuation.definition.string.begin.clojure
;; ^ punctuation.definition.string.end.clojure
;; ^ - string.regexp.clojure
; SYNTAX TEST "Packages/Clojure/Clojure.sublime-syntax"

This comment has been minimized.

@rwols

rwols Oct 24, 2017

Contributor

The point of these syntax test files is that you test the syntax. For example,

"blah \" blah"
; <- punctuation.section
; ^ string.quoted.double

Put your build system on "Automatic" and hit "Build" to run these two tests.

@wbond

This comment has been minimized.

Collaborator

wbond commented Oct 24, 2017

The existing tests should remain. If scope names need to change, a brief explanation of why should be sufficient.

As @rwols mentioned, it appears your additions to the test file doesn't actually add any assertions. From this I presume you weren't running the Syntax Tests build system when developing this, correct? If so, you should have gotten an indication that there were 0 assertions.

[Clojure] addressed cosmetic concerns from PR discussion
* whitespace
* naming conventions
* tests under subfolder

@mitranim mitranim force-pushed the mitranim:master branch from fe51096 to 5a6a5a6 Oct 24, 2017

@mitranim

This comment has been minimized.

Contributor

mitranim commented Oct 24, 2017

Hey guys, thanks for the reviews and instructions!

@jrappen Added whitespace for clarity, moved tests under.

@rwols

Thanks for suggesting PackageDev. It's really cool!

Addressed the cosmetics: junk scope variables, quotes, context naming.

Why entity.name.fn over entity.name.function for lambdas: personally, I prefer keeping lambdas out of the symbol index. Since tail recursion uses recur, fn names are useful only for better stacktraces, and you never use that name outside the function body. It just interferes with search. Personal preference! Receptive to opinions.

@rwols, @wbond

Tests: I focused on covering as many edge cases as possible, since existing tests were wildly insufficient. Is there a way to automatically add missing assertions to the test file? (If not, this would be really cool to have in PackageDev.)

On a tangential note: what does Symbol List do?

@jrappen

This comment has been minimized.

Contributor

jrappen commented Oct 25, 2017

@mitranim

  • How are you running the tests? You can run different tests on your syntax files. Open the syntax file, then from the menu choose "Tools > Build with ...". You can see that you can run:
  • Regarding Symbol List: Open a file, then type CtrlR which gives the same results as typing CtrlP followed by an @. For how to use them compare the metadata files in this repo (*.tmPreferences) for various syntaxes. There are four available settings:
    • showInSymbolList
    • showInIndexedReferenceList
    • showInIndexedSymbolList
    • symbolTransformation
@mitranim

This comment has been minimized.

Contributor

mitranim commented Oct 25, 2017

@jrappen

For now, those tests are for 'manual' reading. I understand how to define and run syntax tests, but prioritised covering all edge cases first, since it's more useful than half a page of assertions that cover like 5%, even if you have to check them manually (IMO). Now the question is: is there a tool to automatically add missing assertions to the test? That would be just perfect.

Pretty sure the syntax file is there. Github collapses the diff because it's huge.

Thanks for explaining the symbol lists. Looks like the Clojure symbol list references a scope (entity.global.clojure) that's unused in both versions of the syntax, and nothing else. Maybe we should just delete it for now?

@jrappen

This comment has been minimized.

Contributor

jrappen commented Oct 26, 2017

@mitranim

I forgot to mention showInIndexedReferenceList (Build 3145 or later).

Compare: SublimeText/PackageDev#165

@wbond

This comment has been minimized.

Collaborator

wbond commented Oct 26, 2017

@mitranim No, there is no tool to automatically generate the assertions. From a logical standpoint they wouldn't be testing much since they would just be using what you've already codified as the scope names in the syntax.

When I have some time, I'll go through some of the scope naming. Hopefully you read http://www.sublimetext.com/docs/3/scope_naming.html before writing the syntax.

In terms of entity.name.fn and the function list: the default syntaxes aren't a place to customize things to your liking. Plus, the correct way to not have certain scopes indexed or in the symbol list is to create a .tmPreferences file in your Packages/User/ folder that turns off indexing/symbol list for those scopes. We'll definitely want to use the standardized scopes in this syntax, especially since so much work went into it.

Unless there is a good reason to delete the old tests, I'd really prefer they be put back. You should be able to leave them as-is and add your own tests afterwards.

The final question I have for now is: did you run the Syntax Tests: Regex Compatibility build system to see if any incompatible regex features were used?

[Clojure] autotests, regex, reify, proxy, other improvements
* added test suite with automatic assertions
* regex support
* reify and proxy support
* corrected scopes for lambda and method names, disabled indexing using symbol settings
* simplified keyword regex, since it was too permissive anyway
* don't match 1-radix integers
@mitranim

This comment has been minimized.

Contributor

mitranim commented Oct 28, 2017

Thanks for all the comments folks! Pushed an update following your suggestions, plus extra corrections:

  • added autotests
  • corrected lambda and method name scopes, banned them from indexing using the "symbol list" file (@jrappen, @wbond, thanks for the hints)
  • restored regex support
  • added reify and proxy support
  • restored the old test file, but without the meta scope assertions

@wbond Yep, it passes the regex compliance test.

Please let me know if you feel the giant test suite is too verbose for maintenance. I felt like comprehensiveness was a priority, on the grounds that the previous tests didn't keep the syntax from being buggy. Most of the rules are there because I broke them at some point while writing the syntax, and future changes could do the same.

Kept the "human-only" suite since it's easier to review on a glance. Maybe should remove that.

Haven't added meta scopes yet. I've seen quite a few syntaxes with flat scopes (1-2 levels everywhere), still unsure.

There might be other lurking issues. Please feel free to make suggestions or edits. I'm at the end of my wits here, might need a few days' break. 😅

@rwols

rwols approved these changes Oct 28, 2017

Starting to look good now from a syntax-file point of view, although I must say again that I don't know much about Clojure.

@@ -534,7 +534,7 @@
; ^^^^^^ string.clojure
; ^ punctuation.section.parens.end.clojure
; ^ punctuation.section.parens.begin.clojure
; ^^^^^^
; ^^^^^^ string.clojure

This comment has been minimized.

@rwols

rwols Oct 31, 2017

Contributor

maybe use string.quoted.double.clojure?

This comment has been minimized.

@mitranim

mitranim Oct 31, 2017

Contributor

rename string.clojurestring.quoted.double.clojure everywhere?

(edit: renamed)

@jrappen

This comment has been minimized.

Contributor

jrappen commented Nov 3, 2017

  • The uuid in *.tmPreferences files is redundant if not used (... by Python files to access them or whatever).
  • While at it, maybe you could fix some of the open clojure issues?
[Clojure] minor corrections and improvements
* braces are no longer labeled as brackets
* removed unused uuid from prefs
* include dot in default word separators (feels better in practice)
@mitranim

This comment has been minimized.

Contributor

mitranim commented Nov 6, 2017

@jrappen

Removed unused uuid.

Looked into the open Clojure issues:

  • #921 looks reasonable; turned out that this syntax uses the stack in a way that makes it difficult to add meta scopes, so this will take some time to figure out; I'll try to look into it later
  • #807 is fixed automatically
  • #795 is fixed automatically
  • #481 is fixed automatically
  • not sure if #582 affects the new syntax; also not sure if the proposed changes are supported by existing color schemes
[Clojure] disable apostrophe auto-pairing in code
* added hotkey to disable apostrophe auto-match in source code
* pairing should work as normal in strings and comments
* the comment syntax rule also captures end-of-line so that apostrophe pairing works at the last cursor position in a comment line
@mitranim

This comment has been minimized.

Contributor

mitranim commented Nov 12, 2017

Additional improvement: disabled apostrophe auto-pairing outside of comments and strings.

[Clojure] keymap niceties
* disable apostrophe auto-pairing in code; keep it enabled in strings and comments
* prevent the inconvenient built-in command `word_wrap` from running

@mitranim mitranim force-pushed the mitranim:master branch from 160efb9 to 1bcffbe Nov 15, 2017

@mitranim

This comment has been minimized.

Contributor

mitranim commented Nov 23, 2017

Corrected detection of n-radix ints. (They no longer accidentally grab non-alphanumeric symbols.)

@mitranim mitranim force-pushed the mitranim:master branch from f53936d to 517779e Nov 27, 2017

mitranim added some commits Dec 5, 2017

[Clojure] various scope corrections
* var defs use `storage.modifier`, not `storage.type`
* in type- and interface-related forms, use `entity.other.inherited-class` for type and interface names
* `declare` uses the appropriate scopes: `storage...` and `entity.name.function.forward-decl`
* comma is marked as punctuation in addition to being a comment
[Clojure] corrections for #dispatch and @deref
* corrected parsing for dispatch macros that don't allow whitespace: #_ #' #"
* added support for ## (symbolic value macro)
* # in #"" no longer receives regex scope; seems more logically consistent with how the language reader works
* removed special case for deref; use the same scope as other reader macros
@wbond

Here are some questions I had after reviewing the current state. I imagine most of them are due to my unfamiliarity with Clojure.

10,20,30
; ^^ constant.numeric.clojure
; ^ comment.punctuation.comma.clojure

This comment has been minimized.

@wbond

wbond Feb 8, 2018

Collaborator

comment.punctuation seems like a funny scope name. Are the commas really comments and not some sort of separator?

This comment has been minimized.

@mitranim

mitranim Feb 10, 2018

Contributor

In Clojure/EDN, commas are parsed as whitespace, i.e. not included in the AST, so yeah, they're comments.

; ## Unaffected
'nil (true) (nil)
; ^ constant.other.symbol.reader-macro.clojure

This comment has been minimized.

@wbond

wbond Feb 8, 2018

Collaborator

Can you explain a little bit about what sort of semantics these constant.other.symbol.reader-macro symbols are? It seems like a funny scope name to me, however I am not familiar with Clojure, and only slightly familiar with lisp-style languages.

This comment has been minimized.

@mitranim

mitranim Feb 10, 2018

Contributor

Come to think of, this scope name is probably wrong. Reader macros are basically unary prefix operators that modify the next piece of AST as it's being read. I didn't want to mark them as regular operators since they work at parse time, not runtime, and it's nice if the color scheme can make a distinction, hence the current name. In many syntaxes, constant.other.symbol receives special highlighting. We should probably pick a different scope, but which?

This comment has been minimized.

@mitranim

mitranim Mar 11, 2018

Contributor

Not sure if keyword.macro.clojure or keyword.operator.macro.clojure makes sense? Looks like reader macros fall under the keyword variety, and I want a scope that's different from keyword.operator but not too Clojure-specific. Ideally something that's already commonly supported in color schemes. (That's what constant.other.symbol does.) What would you suggest?

; ^^^^^^ constant.numeric.clojure
; ^^^^^ constant.numeric.clojure
; ^^^^^^ constant.numeric.clojure
0x1234af 0x1234afN 0X1234AF 0X1234AFN

This comment has been minimized.

@wbond

wbond Feb 8, 2018

Collaborator

What do the various af and n suffixes mean? We have some conventions we've been applying to recent work on syntaxes where we apply scopes to target storage/type info in the numeric literals. My hunch is they may be applicable here also. See https://github.com/sublimehq/Packages/blob/master/Python/syntax_test_python.py#L935-L940 for an example.

This comment has been minimized.

@mitranim

mitranim Feb 10, 2018

Contributor

0x_______ is a hex integer literal, which may include [A-Fa-f]. The N at the end denotes a bigint literal.

This comment has been minimized.

@mitranim

mitranim Feb 10, 2018

Contributor

Ooh I see about storage. By that logic, the N at the end should be storage.type.numeric.bigint.clojure. I think it should still be part of the constant.numeric scope, since it's part of the number literal, so it needs a double scope. Should I make this change?

This comment has been minimized.

@FichteFoll

FichteFoll Feb 12, 2018

Contributor

Yes, that's how it is in Python (and other syntaxes). Also note the punctuation.definition.numeric.hexadecimal.python scopes.

; ^^^^^^^ constant.numeric.clojure
; ^^^^^^^^^^ constant.numeric.clojure
; ^^^^^^^^^^ constant.numeric.clojure
0/10 10/20 30/0

This comment has been minimized.

@wbond

wbond Feb 8, 2018

Collaborator

What does the / represent?

This comment has been minimized.

@mitranim

mitranim Feb 10, 2018

Contributor

In Clojure, N/M, where N and M are integers, is a ratio literal. Ratios are a special numeric data type.

This comment has been minimized.

@FichteFoll

FichteFoll Feb 12, 2018

Contributor

How about adding a punctuation.separator.ratio-literal.clojure or similar scope to the slash?

This comment has been minimized.

@mitranim

mitranim Feb 12, 2018

Contributor

Sure. punctuation.separator.ratio.clojure sounds good to me.

; ^^^^^ constant.numeric.clojure
; ^ punctuation.section.brackets.end.clojure
; ^ punctuation.section.parens.end.clojure
([100.100 200.200])

This comment has been minimized.

@wbond

wbond Feb 8, 2018

Collaborator

Are these floats, or do the . represent something else?

This comment has been minimized.

@mitranim

mitranim Feb 10, 2018

Contributor

These are floats. Like many other tests, this one looks funny because it tests for funny mistakes I made when developing the syntax.

; ^^^ constant.language.keyword.clojure
; ^- constant
; ^^^ constant.language.keyword.clojure
:blah

This comment has been minimized.

@wbond

wbond Feb 8, 2018

Collaborator

Is :blah really a built in keyword?

This comment has been minimized.

@mitranim

mitranim Feb 10, 2018

Contributor

Clojure doesn't have any built-in syntactic keywords as seen in other languages, since it's defined in terms of EDN data structures. The keyword data type is the same as Erlang or Ruby atoms. They're literals similar to strings, interned for performance and used as map keys, enum values, etc.

This comment has been minimized.

@wbond

wbond Feb 12, 2018

Collaborator

In that case they should be constant.other, not constant.language: https://www.sublimetext.com/docs/3/scope_naming.html#constant.

; # Keywords
:! :$ :% :& :* :- :_ := :+ :| :< :> :. :/ :?

This comment has been minimized.

@wbond

wbond Feb 8, 2018

Collaborator

Are these keywords, or more like operators?

This comment has been minimized.

@mitranim

mitranim Feb 10, 2018

Contributor

There are actual valid keywords, which the syntax must support. On a side note, Clojure doesn't have any operators; things like + and = are regular identifiers. (Edit: reader macros can be considered operators.)

This comment has been minimized.

@FichteFoll

FichteFoll Feb 12, 2018

Contributor

There is a keyword scope. Why did you not choose a descendant of that?

Edit: Okay, seems like just anything preceeded by a colon becomes a keyword of some sort. The constant.language does seem more appropriate here than keyword.

This comment has been minimized.

@wbond

wbond Feb 12, 2018

Collaborator

So it would see to me from your description that the semantics of :+ and :blah are the same, right? They should either be scoped the same, or if by convention they are used as "operators" we could scope them as such. I don't really see a reason they should be keywords, however.

This comment has been minimized.

@FichteFoll

FichteFoll Feb 12, 2018

Contributor

Currently they both have the constant.language.keyword.clojure scope.

I don't see any tests against a punctuation scope here, so scoping the colon as punctuation.definition.keyword.clojure sounds like a good addition.

Alternatively, if these are more similar to Ruby's symbol, maybe draw some inspiration for scope names from that?

This comment has been minimized.

@mitranim

mitranim Feb 12, 2018

Contributor

AFAICT, Ruby symbols are exactly like Clojure keywords. Looks like we all agree to follow the example and use constant.other.keyword.clojure for the entire constant and punctuation.definition.keyword.clojure for the colon.

@mitranim

This comment has been minimized.

Contributor

mitranim commented Feb 10, 2018

One issue that I see with the current implementation of the PR is that it uses the stack in a way that makes it difficult to add meta scopes. Can address this if the need arises, but it just felt like a low priority.

mitranim added some commits Feb 10, 2018

[Clojure] various scope improvements
* added miscellaneous scopes to number literals: signs, dots, slashes, radix modifiers, storage modifiers
* reader macros now have `keyword.operator.macro` scopes rather than `constant.other.symbol`
* keywords now have `constant.other.keyword` scopes rather than `constant.language.keyword`
@mitranim

This comment has been minimized.

Contributor

mitranim commented Mar 11, 2018

Addressed the comments above.

@mitranim

This comment has been minimized.

Contributor

mitranim commented Apr 15, 2018

Pretty satisfied with how it works. Not strictly "done" (see the above comments on meta scopes), but haven't felt the need for more changes in the past few months. Might be stable enough to move forward with the PR for the sake of other Sublime/Clojure users, all 3 of them (?).

@wbond

This comment has been minimized.

Collaborator

wbond commented Apr 16, 2018

I am going to hold off merging any significant work right now since the dev cycle is almost complete. We'll get this merged in at the beginning of the next dev cycle.

My only concern, is based on comments, it sounds as if there may be somewhat of a regression around function definitions. Is it just parameters that aren't highlighted, or the whole function definitions themselves? I think it is worth extra effort to support Goto Definition if this currently doesn't track the definitions of functions. Please excuse my lack of memory in regards to this PR.

@mitranim

This comment has been minimized.

Contributor

mitranim commented Apr 16, 2018

Yeah I just don't know the process, was wondering.

Symbol indexing works perfectly. I just banned inline lambdas from indexing, since you never want to "goto" them anyway, and they'd clog the ⇪⌘R search.

@wbond

I am about to merge this in so that we can get feedback and bug reports during the next dev cycle.

These questions and comments are about the supporting files for the package.

"word_separators": "/\\()[]{}\"'`,:;@#^~.",
"smart_indent": false,
"detect_indentation": false,
"tab_size": 1,

This comment has been minimized.

@wbond

wbond Jun 18, 2018

Collaborator

This seems funny to me. Why would the tab size be hardcoded to 1 for all Clojure files?

This comment has been minimized.

@mitranim

mitranim Jun 18, 2018

Contributor

Most Clojure code follows the same indentation convention, which sometimes involves 1-space indents. The Lispindent package automates this style, but when refactoring, you often need to shift a block by 1 space, and it's needlessly painful if the indentation is not set to 1.

Example from the test file: [1].

This comment has been minimized.

@mitranim

mitranim Jun 18, 2018

Contributor

The most common reason for indent 1 is when writing nested data structures, such as Hiccup-style markup:

(def page []
  [:div.class0
   [:div.class1
    [:div.class2
     ...]]])
"smart_indent": false,
"detect_indentation": false,
"tab_size": 1,
"draw_indent_guides": false

This comment has been minimized.

@wbond

wbond Jun 18, 2018

Collaborator

Is this related to the tab size of 1?

This comment has been minimized.

@mitranim

mitranim Jun 18, 2018

Contributor

Yeah. There's a reason for these settings.

  • smart_indent doesn't play well with Lisp code
  • detect_indentation can get confused by traditional Clojure indentation, and there's very rarely any reason for indent other than 1
  • draw_indent_guides creates needless noise with indent 1
@@ -0,0 +1,26 @@
[

This comment has been minimized.

@wbond

wbond Jun 18, 2018

Collaborator

This file should use tabs for indentation

This comment has been minimized.

@mitranim

mitranim Jun 18, 2018

Contributor

Fixed.

mitranim added some commits Jun 18, 2018

@FichteFoll

This comment has been minimized.

Contributor

FichteFoll commented Jun 20, 2018

If most code is formatted that way, the automatic detection would be able to catch this, wouldn't it? I'd be very wary of overriding indentation settings for all users, unless the format basically requires you to use it (kind of like YAML).

However, if you believe in good faith that no sane user should ever use any other settings than the ones you provided (because they are very troublesome to work with or outright don't work at all), I certainly do see reasons for having them.

@mitranim

This comment has been minimized.

Contributor

mitranim commented Jun 21, 2018

Automatic indent detection is based on the first indented lines. Most Clojure files start with this:

(ns my-domain.my-namespace
  "Namespace docstring"
  (:require
    [namespace0]
    [namespace1])
  (:import
    [class0]
    [class1]))

Conventionally, the next line inside () is intended by 2 spaces, whereas the next line inside [] or {} is indented by 1 space. Automatic detection would always pick 2 spaces, which would be incorrect.

The main reason why I feel 1 space is more convenient is refactoring. Sometimes you need to shift a block of code by an uneven number of spaces due to the aforementioned conventions. If the block doesn't have any newlines in it, multi-cursor editing works fine. But if the block does have newlines and you're shifting to the left, you can't just multi-cursor: it would delete a newline and then delete something extra.

This is just my experience, based on the indentation convention that appears to be dominant in Clojure. If someone finds this default inconvenient, they're welcome to complain and we'll change it.

@wbond

This comment has been minimized.

Collaborator

wbond commented Jul 9, 2018

I'm keen on getting this in so that it gets testing from other Clojure users during the next dev cycle. That said, I'm not convinced about the indentation settings tweak. Could you remove those and propose them in another PR, and hopefully we can get some feedback from other ST Clojure users in the future?

@mitranim

This comment has been minimized.

Contributor

mitranim commented Jul 10, 2018

Glad to hear this is getting progress.

No problem with removing the tweak, but are you sure going 2 → 1 is the right order? I assume that for most users, if 2 is inconvenient, they won't provide feedback saying 1 is better; nobody will even try 1 because it's so radical. Going 1 → 2, if folks find 1 inconvenient, they're somewhat more likely to complain, and we're more likely to receive feedback.

@ItamarShDev

This comment has been minimized.

ItamarShDev commented Jul 11, 2018

I will be glad to help feedback-wise.
I use clojure daily at work :)

@rwols

This comment has been minimized.

Contributor

rwols commented Sep 17, 2018

@ItamarShDev have you tried this diff on your codebase?

@wbond wbond merged commit 40a9178 into sublimehq:master Nov 28, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@wbond

This comment has been minimized.

Collaborator

wbond commented Nov 28, 2018

Thanks for all of your work on this @mitranim, it seems like a nice enhancement to the syntax. Hopefully we can get some feedback from other users in this dev cycle.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment