Page MenuHomePhabricator

ArcanistTextLinter: Option to support UTF8
Needs ReviewPublic

Authored by malandrew on Jun 8 2016, 11:30 PM.
Tags
None
Referenced Files
F14083583: D16085.id.diff
Sat, Nov 23, 4:50 AM
Unknown Object (File)
Wed, Nov 20, 7:52 AM
Unknown Object (File)
Sat, Nov 16, 5:38 AM
Unknown Object (File)
Tue, Nov 12, 7:39 PM
Unknown Object (File)
Tue, Nov 12, 12:00 AM
Unknown Object (File)
Fri, Nov 8, 12:40 PM
Unknown Object (File)
Mon, Nov 4, 2:28 PM
Unknown Object (File)
Wed, Oct 30, 11:52 PM
Subscribers

Details

Summary

This adds an option to the text linter for allowing UTF-8 characters
in text files. This was added to support the following (but is not limited to
the following characters):

├── foo
├── bar
└── baz

The default behavior of not allowing UTF8 characters is preserved.`

Test Plan

Added the above example text from Summary into the README
temporarily and added "text.allow-utf8": true to the .arclint temporarily
and then ran ./bin/arc lint to confirm UTF8 characters do not raise warning.
I ignored the snakecase lint warnings because the entire style inside
ArcanistTextLinter is camelCase.

Diff Detail

Repository
rARC Arcanist
Branch
utf8-text-linter
Lint
Lint Passed
Unit
Test Failures
Build Status
Buildable 12574
Build 15956: arc lint + arc unit

Unit TestsFailed

TimeTest
1,631 msArcanistJSHintLinterTestCase::Unknown Unit Message ("")
In 'jshint.lint-test', expected lint to raise error on line 9 at char 0, but no error was raised. Actually raised: warning at line 3, char 8: W033 JSHintW033 error at line 7, char 1: E019 JSHintE019
538 msArcanistJscsLinterTestCase::Unknown Unit Message ("")
In 'curly-brace.lint-test', expected lint to raise error on line 2 at char 27, but no error was raised. Actually raised: error at line 2, char 3: JSCS JSCS error at line 2, char 3: JSCS JSCS
232 msArcanistPEP8LinterTestCase::Unknown Unit Message ("")
EXCEPTION (CommandException): Command failed with error #1! COMMAND 'pep8' --version
57 msArcanistPEP8LinterTestCase::Unknown Unit Message ("")
Assertion failed, expected values to be equal (at ArcanistLinterTestCase.php:168): Command failed with error #1! COMMAND 'pep8' --version
81 msArcanistPyFlakesLinterTestCase::Unknown Unit Message ("")
Assertion failed, expected values to be equal (at ArcanistLinterTestCase.php:168): Command failed with error #1! COMMAND 'pyflakes' --version
View Full Test Results (6 Failed · 32 Passed · 16 Skipped)

Event Timeline

malandrew retitled this revision from to ArcanistTextLinter: Option to support UTF8.
malandrew updated this object.
malandrew edited the test plan for this revision. (Show Details)
malandrew added reviewers: epriestley, avivey.

I ignored the snakecase lint warnings because the entire style inside ArcanistTextLinter is camelCase.

https://secure.phabricator.com/book/phabcontrib/article/php_coding_standards/#case-and-capitalization

Can I add ¯\_(ツ)_/¯ to the project README and update .arclint to support UTF8?

@chad So I should ignore the convention currently in the file for all new additions?

I'm not sure what you're asking. Lint is raising errors and you should fix them, lint is correct, per convention. If the file is incorrect somewhere, I can't find it.

malandrew edited edge metadata.
  • Fix lint warnings

@chad Forgive me if I misunderstood, but is that document suggesting that I need to pay to get a diff reviewed and accepted?

I understand paying to get a new feature implemented or getting a diff reviewed ASAP, but it strikes me as odd to pay if I've provided free labor to implement it myself and the feature is a very reasonable and broadly useful feature (in addition to my use case, my diff also allows customers in many foreign languages to use characters from their language in their READMEs and other text documents). I could see paying as reasonable if I want this done ASAP, but I just want to get an idea of when this might be accepted.

That link says you should not ask "when are you going to do X", because the answer is "we don't know".

Sorry I read that differently. I though that was only addressing when you might build a feature, not when you might review a feature. In my mind (having maintained projects myself), asking a maintainer to build something new is very different from asking for review of something someone has already implemented. The former is the request from someone who takes and doesn't give back and the latter is someone who is (usually) working towards the same goal of making something better and can be a valuable productive member of the community if their interest in the project is nurtured.

Should I assume that the answer to "When will you review a diff (for some feature)?" is the same as "When will you build (some feature)?"? Or are diffs treated differently than feature requests?

Thanks for that second document. I didn't find it earlier.

I'm just the designer, so I can't review this for you. I don't have any idea when it may be reviewed, but it won't likely be immediate, which the documents @avivey listed should explain. Since we're only 2 people full-time on a large project, we do our best to stay focused on current roadmap items and only look into non-priority stuff as we have time.

In general contributions are not free for us. That is, when we take code, we're expressly taking over ownership, maintenance, documentation, and support for life. We don't expect you to maintain the code moving forward, or be in IRC answering questions. In most cases support is significantly more in cost than the original code written. We do love contributions though! But we'll usually be a bit more thoughtful and thorough in what we bring upstream due to these costs.

We generally want tasks opened first before code so we can decide if code is even needed, if it should wait for other infrastructure work, or if we should do it ourselves given complexity.

You may find T10038 interesting as well.

@avivey Maniphest ticket filed: T11150

@chad thanks for the information and the link to that task.