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

Deprecate Text.Sexp #591

Merged
merged 5 commits into from
Jun 12, 2018
Merged

Deprecate Text.Sexp #591

merged 5 commits into from
Jun 12, 2018

Conversation

ujihisa
Copy link
Member

@ujihisa ujihisa commented Jun 11, 2018

See also: #587 (comment)

* `git mv autoload/vital/__vital__/Text/* autoload/vital/__vital__/Deprecated/Text/`
* `git mv doc/vital/Text/Sexp.txt doc/vital/Deprecated/Text/Sexp.txt`
* `git mv test/Text/Sexp.vim test/Deprecated/Text/Sexp.vim`
tyru
tyru previously requested changes Jun 11, 2018
doc/vital.txt Outdated
@@ -209,7 +209,7 @@ LINKS *Vital-links*
|vital/Text/LTSV.txt| LTSV library.
|vital/Text/Lexer.txt| lexer library.
|vital/Text/Parser.txt| parser library.
|vital/Text/Sexp.txt| S-Expression parser.
|vital/Deprecated.Text.Sexp.txt| S-Expression parser.
Copy link
Member

Choose a reason for hiding this comment

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

dot should be slash.

Copy link
Member Author

Choose a reason for hiding this comment

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

ウギャー一斉置換したときにがばっとミスってました。あぶなすぎる

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed all 3 at 1b117a9

README.md Outdated
@@ -77,7 +77,7 @@ Module | Description
[Text.LTSV](doc/vital/Text/LTSV.txt) | LTSV library
[Text.Lexer](doc/vital/Text/Lexer.txt) | lexer library
[Text.Parser](doc/vital/Text/Parser.txt) | parser library
[Text.Sexp](doc/vital/Text/Sexp.txt) | S-Expression parser
[Deprecated.Text.Sexp](doc/vital/Deprecated.Text.Sexp.txt) | S-Expression parser
Copy link
Member

Choose a reason for hiding this comment

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

dot should be slash.

@@ -1,27 +1,27 @@
*vital/Text/Sexp.txt* S-Expression parser.
*vital/Deprecated.Text.Sexp.txt* S-Expression parser.
Copy link
Member

Choose a reason for hiding this comment

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

dot should be slash.

thinca
thinca previously requested changes Jun 11, 2018
README.md Outdated
@@ -77,7 +77,7 @@ Module | Description
[Text.LTSV](doc/vital/Text/LTSV.txt) | LTSV library
[Text.Lexer](doc/vital/Text/Lexer.txt) | lexer library
[Text.Parser](doc/vital/Text/Parser.txt) | parser library
[Text.Sexp](doc/vital/Text/Sexp.txt) | S-Expression parser
[Deprecated.Text.Sexp](doc/vital/Deprecated/Text/Sexp.txt) | S-Expression parser
Copy link
Member

Choose a reason for hiding this comment

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

Deprecated 名前空間以下のモジュールは現状 README や doc/vital.txt のモジュール一覧に記載されていないので、合わせた方が良いかと。

Copy link
Member Author

Choose a reason for hiding this comment

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

True. Fixed at 949f6ed. thx!

Copy link
Member

Choose a reason for hiding this comment

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

help も頼むー。

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops, why didn't I notice that? Thanks @thinca, good catch! fixed at 33847a7


*Vital.Text.Sexp* is a S-Expression parsing library. For performance it uses
*Vital.Deprecated.Text.Sexp* is a S-Expression parsing library. For performance it uses
lua, so the Vim that runs this needs |+lua|.
Copy link
Contributor

Choose a reason for hiding this comment

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

How about adding a notation that this module was deprecated? (though it's obvious from its module name...)

Copy link
Member Author

Choose a reason for hiding this comment

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

That's also an option, but I feel that it's verbose to repeat there. (Alternative option would be to insert warning message when you use, but before that the plugin author need to rename the module, so it's still verbose)

@ujihisa ujihisa dismissed stale reviews from thinca and tyru June 12, 2018 03:54

completed

@ujihisa
Copy link
Member Author

ujihisa commented Jun 12, 2018

Thanks @tyru @thinca @rhysd !

@ujihisa ujihisa merged commit 1c4b777 into master Jun 12, 2018
@ujihisa ujihisa deleted the sexp branch June 12, 2018 03:55
tyru added a commit that referenced this pull request Jun 13, 2018
tyru added a commit that referenced this pull request Jun 13, 2018
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.

4 participants