naming.cell.parse implementation #281

qfox · 2018-02-05T20:56:14Z

Closes #276

cc @Vittly

Vittly · 2018-02-06T09:01:24Z

Any tests ?

qfox · 2018-02-06T10:46:53Z

This code was in Sublime's cache since October... Better here in branch than in my local cache.

Anyway it's a nice start I guess to make what you need.

qfox · 2018-04-06T00:19:12Z

cc @blond @tadatuta

Guess it's time to review.

awinogradov

Please add implementation for nested modern schema with platform folder inside block.

Vittly · 2018-04-09T12:55:44Z

packages/cell/index.js

@@ -287,7 +287,8 @@ module.exports = class BemCell {
     * @returns {boolean} A Boolean indicating whether or not specified entity is instance of BemCell.
     */
    static isBemCell(cell) {
-        return !!cell && Boolean(cell.__isBemCell__);
+        const C = cell && cell.constructor;
+        return C === this || Boolean(C && cell.__isBemCell__ && C !== Object);


What is C ?

C is Consistency!

For what is to Boolean ?

это выражение и так априори boolean

Vittly · 2018-04-09T13:01:44Z

packages/naming.cell.match/cell-match.js

+const ALPHANUM_RE = '[a-z][\\w\\-]*';
+const resc = s => String(s).replace(/[\\^$*+?.()|[\]{}]/g, '\\$&');
+
+const buildPathParseMethod = ({ fs: { pattern, delims: dd = {}, scheme = 'nested' }, delims, wordPattern: wp = ALPHANUM_RE }) => {


Seems like we need a default preset. Now some defaults are located inside "naming.cell.match" package. It is not obvious

Vittly · 2018-04-09T13:03:43Z

packages/naming.cell.match/cell-match.js

+    const separation = pathPatternParser(pattern);
+
+    const dElem = 'elem' in dd ? dd.elem : (delims.elem || '__');
+    const dMod = 'mod' in dd ? dd.mod : (Object(delims.mod).name || (typeof delims.mod === 'string' && delims.mod) || '_');


This code is very similar to this. Not right

Need to move it to BemNamingConvention container too.

Fix planned in #304

Vittly · 2018-04-09T13:06:36Z

packages/naming.cell.match/cell-match.js

+
+    const entityRe = ({
+        flat: [`(?:(${wp})(?:/(${ALPHANUM_RE})`, ')?)?', (entity, { dir }) => (entity.block === dir)],
+        nested: [`(?:(${wp}(?:|/${dElem}${wp})?(?:|/${dMod}${wp})?)(?:/(${ALPHANUM_RE})`, ')?)?', (entity, { dir }) => {


Too complex for me. Can we write here a helper? Now I need to take a pencil/paper and start count braces

Why you should count it? Who or what forces you to do it?

Guess we need to move it out to separate package along with scheme path builders.

Probably it should be #304 for a while.

Vittly · 2018-04-09T13:07:39Z

packages/naming.cell.match/cell-match.js

+            let i = 1;
+            return entity.block === parts[0] &&
+                (!entity.elem || (parts[i++] === dElem + entity.elem)) &&
+                (!entity.mod || (parts[i++] == dMod + entity.mod.name));


Very hard to read this

That's why we have tests. And me.

Vittly · 2018-04-09T13:22:26Z

packages/naming.cell.match/cell-match.test.js

+const safeEval = require('node-eval');
+
+const BemCell = require('@bem/sdk.cell');
+const { origin } = require('@bem/sdk.naming.presets');


We use destructuring assignment but we aware of spread in object expression (using Object.assgin). Why?

"package.json" declares node 4 and it has no support of destructuring http://node.green/#ES2015-syntax-destructuring--assignment

Well, I guess if we need node 4 then we need to fix it.

I'm just living in the future ;-(

We decided to drop 4.x support so it can be left as is.

Vittly · 2018-04-09T13:26:48Z

packages/naming.cell.match/cell-match.test.js

+describe('naming.cell.match', () => {
+    for (const [dTitle, [match, its]] of Object.entries({
+        'flat / origin': [flatOriginMatch, rawses`
+            reject invalid                 → blocks                        → { isMatch: false }


Don't why is it better than classic describe/it

Just to reduce amount of code and increase readability of tests.

Vittly · 2018-04-09T13:28:44Z

packages/naming.cell.match/cell-match.test.js

+            rejects alien block mod2       → dd.blocks/qq/bb_m_v.t        → { isMatch: false }
+            rejects alien elem             → dd.blocks/qq/bb__e.t        → { isMatch: false }
+            rejects alien elem mod         → dd.blocks/qq/bb__e_m.t   → { isMatch: false }
+            rejects alien elem mod2        → dd.blocks/qq/bb__e_m_v.t → { isMatch: false }


Something went wrong with tabs before the arrow

They are spaces ;-D

Vittly · 2018-04-09T13:30:48Z

packages/naming.cell.match/cell-match.test.js

+            reject invalid block           → button/button → { cell: null, isMatch: false }
+            match partial block path       → blocks → { cell: null, isMatch: true }
+            parse typical block path       → blocks/blocks.css → { cell: { layer: 'common', block: 'blocks', tech: 'css' } }
+            parse typical block in layer   → blocks/[email protected] → { cell: { layer: 'ios', block: 'blocks', tech: 'css' } }


Wow! So here is a modern scheme. Want to see it nested

Something like:

bb/bb.t # previously common.blocks/bb/bb.t bb/[email protected] # previously desktop.blocks/bb/bb.t bb/[email protected] # previously touch.blocks/bb/bb.t bb/_e/bb_e.t # previously common.blocks/bb/_e/bb_e.t bb/_e/[email protected] # previously desktop.blocks/bb/_e/bb_e.t bb/__m/bb__m.t # previously common.blocks/bb/__m/bb__m.t bb/__m/[email protected] # previously desktop.blocks/bb/__m/bb__m.t

In the end I want to support this: bem-sdk-archive/bem-walk#77 (comment)

Vittly · 2018-04-09T13:34:25Z

packages/naming.cell.match/package.json

+  },
+  "devDependencies": {
+    "@bem/sdk.naming.presets": "*"
+  },


Cannot fine "node-eval", "mocha", "nyc", "chai" in your deps

They are in root package.json, shared between subpackages

Yeti-or

my eyes are bleeding
but it's seems to work fine

qfox · 2018-06-22T18:08:15Z

packages/naming.cell.match/cell-match.js

+ * Stringifier generator
+ *
+ * @param {BemNamingConvention} conv - naming, path and scheme
+ * @returns {function(BemCell): string} converts cell to file path


na vhod string

packages/naming.cell.match/cell-match.js

+                r[key] = res[i+1];
+            }
+            return r;
+        }, {});


qfox · 2018-06-22T18:14:29Z

packages/naming.cell.match/cell-match.js

+
+        const entity = obj.entity && entityParse(obj.entity);
+        if (entity && !isValid(entity, obj)) {
+            return { cell: null, isMatch: false };


vynesti naverh peredachu entityParse

qfox · 2018-06-22T18:16:22Z

packages/naming.cell.match/cell-match.js

+        }
+
+        const cell = entity ? BemCell.create(Object.assign({ layer }, obj, { entity })) : null;
+        return !obj.rest ? { cell, isMatch: true } : { cell, isMatch: false, rest: obj.rest };


veged protiff

tadatuta · 2018-06-29T10:14:27Z

тесты попадали из-за линтера и es6
появились заскипы — это точно норм?

birhoff · 2018-06-29T22:36:21Z

packages/naming.cell.match/cell-match.js

+const resc = s => String(s).replace(/[\\^$*+?.()|[\]{}]/g, '\\$&');
+
+const SCHEMES = {
+    flat: ({ wp }) => [


empty arsgs

birhoff · 2018-06-29T22:47:59Z

packages/naming.cell.match/cell-match.js

+    /**
+     * [description]
+     * @param  {[type]} relPath [description]
+     * @return {[type]}         [description]


nice docs bro

birhoff · 2018-06-29T22:55:18Z

packages/naming.cell.match/cell-match.test.js

+                ...safeEval(`(${rawExpected})`)
+            };
+            expected.cell = expected.cell ? BemCell.create(expected.cell).id : null;
+            expected.isMatch = expected.isMatch === false ? false : Boolean(expected.isMatch || expected.cell);


this is strange

Не, всё норм. Можно переписать условие, но иногда isMatch не указан вообще, короче, так надо

birhoff · 2018-06-29T22:56:38Z

packages/walk/lib/index.js

+    // walk(['node_modules/islands', 'contribs/search-header'], config);
+
+    // TODO: Warn about old using old walkers
+    // assert(defaults.scheme, 'sdk/walk: Please stop using old API');


birhoff · 2018-06-29T22:56:45Z

packages/walk/lib/index.js

    const defaults = options.defaults || {};
+    // const isLegacyDefaults = 'scheme' in defaults;


birhoff · 2018-06-29T22:56:52Z

packages/walk/lib/index.js

-    const defaultWalker = (typeof defaults.scheme === 'string' ? walkers[defaults.scheme] : defaults.scheme)
-        || walkers.nested;
+    const defaultNaming = defaults.naming || {};
+    // const defaultsNamingFs


birhoff · 2018-06-29T22:58:56Z

packages/walk/lib/index.js

+        });
+
+        // const scheme = config && config.scheme;
+        const walker = walkers.sdk; // typeof scheme === 'string' ? walkers[scheme] : (scheme || defaultWalker);


birhoff · 2018-06-29T23:01:56Z

packages/walk/test/core/defaults.test.js

@@ -13,7 +13,7 @@ const mockFs = require('mock-fs');

 const walkers = require('../../lib/walkers');

-describe('core/defaults', () => {
+describe.skip('core/defaults', () => {


birhoff · 2018-06-29T23:02:01Z

packages/walk/test/core/walkers.test.js

@@ -13,7 +13,7 @@ const mockFs = require('mock-fs');

 const walkers = require('../../lib/walkers');

-describe('core/walkers', () => {
+describe.skip('core/walkers', () => {


birhoff

very shitty docs
code inconsistency es3/es6
jsdoc lint?

fixed

tadatuta mentioned this pull request Feb 6, 2018

Организация блоков в пределах уровня bem-site/bem-forum-content-ru#1372

Open

qfox force-pushed the qfox.naming-cell-parse branch from f98752f to b7f8070 Compare March 24, 2018 15:29

qfox force-pushed the qfox.naming-cell-parse branch 2 times, most recently from c21fb42 to d48e84e Compare April 6, 2018 00:00

qfox force-pushed the qfox.naming-cell-parse branch from 05bdbce to bcadc1d Compare April 6, 2018 00:32

awinogradov reviewed Apr 6, 2018

View reviewed changes

Vittly reviewed Apr 9, 2018

View reviewed changes

Yeti-or approved these changes Apr 26, 2018

View reviewed changes

qfox commented Jun 22, 2018

View reviewed changes

packages/naming.cell.match/cell-match.js

r[key] = res[i+1];

}

return r;

}, {});

This comment was marked as outdated.

Sign in to view

qfox commented Jun 22, 2018

View reviewed changes

qfox force-pushed the qfox.naming-cell-parse branch from 22ab728 to bdf1a05 Compare June 29, 2018 01:03

birhoff reviewed Jun 29, 2018

View reviewed changes

birhoff previously requested changes Jun 29, 2018

View reviewed changes

qfox force-pushed the qfox.naming-cell-parse branch 3 times, most recently from 993718e to 050ebab Compare June 30, 2018 19:52

qfox force-pushed the qfox.naming-cell-parse branch from 050ebab to 90334d3 Compare June 30, 2018 23:45

qfox added 6 commits July 2, 2018 01:09

feat(naming.cell.match): initial implementation

42eefb5

fix(cell): rely on constructor in isBemCell

9553feb

fix(entity-name): rely on constructor in isBemEntityName

74224de

docs: simplify readme

8aafb76

feat(naming.presets): create now respects fs field in convention

6eeadc3

feat(naming.presets): legacy preset, 'blocks' dir, user defaults

09a232a

qfox force-pushed the qfox.naming-cell-parse branch from 94b5c64 to 89c9691 Compare July 1, 2018 22:10

qfox added 2 commits July 2, 2018 01:46

feat(walk): sdk cell match and presets support

187647d

chore: drop support for nodes 4.x and 6.x, add 10.x

76f33f2

qfox force-pushed the qfox.naming-cell-parse branch from 89c9691 to 76f33f2 Compare July 1, 2018 22:46

qfox merged commit c46110d into master Jul 1, 2018

qfox deleted the qfox.naming-cell-parse branch July 1, 2018 22:50

qfox mentioned this pull request Jul 1, 2018

Nested-lite scheme for walk #273

Closed

		const defaults = options.defaults \|\| {};
		// const isLegacyDefaults = 'scheme' in defaults;

naming.cell.parse implementation #281

naming.cell.parse implementation #281

Conversation

qfox commented Feb 5, 2018 • edited Loading

Vittly commented Feb 6, 2018

qfox commented Feb 6, 2018

qfox commented Apr 6, 2018

awinogradov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qfox Apr 11, 2018 • edited Loading

Choose a reason for hiding this comment

Yeti-or left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment was marked as outdated.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tadatuta commented Jun 29, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

birhoff left a comment

Choose a reason for hiding this comment

qfox commented Feb 5, 2018 •

edited

Loading

qfox Apr 11, 2018 •

edited

Loading