Implement parse method #9

tadatuta · 2016-10-07T20:12:57Z

No description provided.

tadatuta · 2016-10-07T20:13:13Z

qfox

А еще: почему parse?

qfox · 2016-10-07T20:14:56Z

lib/schemes/flat.js

+        var naming = bemNaming(options.naming),
+            splittedPath = path.split('.');
+
+        return {


Can we use here BemUnnamedObject? To reduce hidden classes quantity

Object like {entity: {block, elem, mod}, tech: ?String} has no name atm so it's BemUnnamedObject.
You are free to call it somehow, but not BemEntityTech and not Tenorok.

I don't understand profit of such naming so such simple things. Can't we refactor it later when we end up with some name and found real cases for it?

Can't we refactor it later when we

Yes, we can.

end up with some name

We decided to call it BemCell or BemCytus for now

and found real cases for it?

https://github.com/bem-sdk/bem-decl/blob/master/lib/normalize.js#L13
https://github.com/bem-sdk/bem-graph/blob/master/lib/vertex.js#L7
etc.

qfox · 2016-10-07T20:15:37Z

lib/schemes/nested.js

+        options || (options = {});
+        var naming = bemNaming(options.naming),
+            splittedPath = str.split(path.sep).filter(Boolean),
+            file = splittedPath.pop(),


basename = path.basename(str);

I need mutated splittedPath bellow

qfox · 2016-10-07T20:17:16Z

lib/schemes/nested.js

+
+        // support for paths without filename, e.g. 'b1/__e1/'
+        if (!entity) {
+            entity = naming.parse(splittedPath.join('') + splittedFile[0]);


what exactly confuses you?

Why we should parse directories as entities? Any real life case for that?

cd level1/b1/__e1 bem create _m1_v1.css # b1__e1_m1_v1.css

What about

cd level1/b1/b2/b3 bem create _m1_v1.css # b1b2b3_m1_v1.css

nice catch 👍

qfox · 2016-10-07T20:20:27Z

lib/schemes/nested.js

+
+        return {
+            entity: entity,
+            tech: splittedFile[1]


splittedFile.slice(1).join('.');

qfox · 2016-10-07T20:22:21Z

test/test.js

+    describe('default', function() {
+        it('should return path + tech', function() {
+            expect('a/a.js')
+                .eql(scheme().path({ block: 'a' }, 'js'));


that's exactly the way to make it render err messages right

Oh, you are right! And it's really strange 'cos I don't like the way it's done and I remember that when it was written intuitively actual and expected were swapped in error messages.
Will toggle them, thanks!

qfox · 2016-10-07T20:24:08Z

lib/schemes/flat.js

@@ -6,5 +6,15 @@ module.exports = {
        var naming = bemNaming(options.naming);


Well, I think this is not optimal to create the same thing many times per build.

but each level may have its own naming settings

Pass in naming object instead of creating it on each call

https://github.com/bem-sdk/bem-naming/pull/134

qfox · 2016-10-07T20:24:45Z

lib/schemes/flat.js

+
+        return {
+            entity: naming.parse(splittedPath[0]),
+            tech: splittedPath[1]


slice(1).join('.')

qfox · 2016-10-07T20:26:04Z

lib/schemes/flat.js

+    parse: function(path, options) {
+        options || (options = {});
+        var naming = bemNaming(options.naming),
+            splittedPath = path.split('.');


Can we check here that path is valid entity file? At least, can we get path.basename?

you mean something like

if (path.basename(str) !== str) return;

?

Looks like... ;-( Can't imagine something better atm.

qfox · 2016-10-07T20:27:29Z

test/test.js

+            expect(expected).eql(scheme().parse('b1__e1'));
+            expect(expected).eql(scheme().parse('b1/__e1'));
+            expect(expected).eql(scheme().parse('b1/__e1/'));
+            expect(expected).eql(scheme().parse('b1/__e1/b1__e1'));


each assertion should be in separate case ;-(

sound like too much bureaucracy :)

Just each assertion should describe a single case with some title.

А еще: почему parse?

А какие будут предложения?

fetch? ну не знаю.

tadatuta · 2016-10-07T23:38:07Z

🆙

a-x- · 2017-02-14T20:02:37Z

lib/schemes/flat.js


 module.exports = {
    path: function(entity, tech, options) {
        options || (options = {});
        var naming = bemNaming(options.naming);

        return naming.stringify(entity) + '.' + tech;
+    },
+    parse: function(str, options) {


a-x- · 2017-02-15T00:21:12Z

lib/schemes/nested.js

+        return {
+            entity: entity,
+            tech: tech
+        };


my implementation:

function parseBemPath(filePath) { const filePath_ = path.normalize(filePath); const cell = _parseComplexBemPath(filePath_); // With directories-technologies // null is a signal 'incorrect cell found' return cell || cell !== null && _parseSimpleBemPath(filePath_) || null; }

function _parseComplexBemPath(filePath) { const chunks = filePath.split(path.sep); const block = chunks[1]; const secondChunk = splitAtFirst(chunks[2], '.'); // E.g. ['suggest2', 'priv.js'] if(!secondChunk[1]) { // Directory-technology, e.g. button2. < examples >/ return; // skip, go to next processor } const cell = BemCell(secondChunk); return cell && !compareEntities(cell.entity, {block: block}).length ? cell : null; }

function _parseSimpleBemPath(filePath) { const name = path.basename(filePath); const cellFromName = BemCell(splitAtFirst(name, '.')); if(!cellFromName) { return null; } const entityFromDir = parseDirAsBemCell(path.dirname(filePath)); const diff = compareEntities(cellFromName.entity, entityFromDir); return !diff.length || _.isEqual(diff, ['modVal']) ? cellFromName : null; }

compareEntities
parseDirAsBemCell
splitAtFirst
это какие то твои хелперы?

@a-x- а pr прислать ты не хочешь ?

yes, it's mine

yep, i can, but i afraid it useless and what would be changed if i do?

you can see all of helpers in the internal pr: lego/islands-ci-helpers/pull/233

Yeti-or · 2017-02-15T16:51:37Z

lib/schemes/flat.js

@@ -1,10 +1,25 @@
-var bemNaming = require('bem-naming');
+var path = require('path'),
+    bemNaming = require('bem-naming');


@bem/naming

Yeti-or · 2017-02-15T16:54:15Z

test/test.js

+
+            expect(scheme().parse('b1__e1')).eql(expected);
+            expect(scheme().parse('b1/__e1')).eql(expected);
+            expect(scheme().parse('b1/__e1/')).eql(expected);


Are you sure that it is a block_elem, but not block ?

Yeti-or · 2017-02-15T16:54:57Z

lib/schemes/nested.js

+            if (i > 2) { return; }
+        }
+
+        return {


result should be bem-cell

tadatuta added the in progress label Oct 7, 2016

qfox suggested changes Oct 7, 2016

View reviewed changes

Implement parse method

ed3be71

tadatuta force-pushed the feature/parse branch from 00fc3ee to ed3be71 Compare October 7, 2016 23:26

tadatuta mentioned this pull request Oct 10, 2016

partial-bemslug support bem-tools/bem-tools-create#12

Open

a-x- reviewed Feb 15, 2017

View reviewed changes

Yeti-or suggested changes Feb 15, 2017

View reviewed changes

Yeti-or mentioned this pull request May 6, 2017

Implement parse method #18

Open

		@@ -6,5 +6,15 @@ module.exports = {
		var naming = bemNaming(options.naming);

Implement parse method #9

Are you sure you want to change the base?

Implement parse method #9

Conversation

tadatuta commented Oct 7, 2016

tadatuta commented Oct 7, 2016

qfox left a comment

Choose a reason for hiding this comment

qfox Oct 7, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qfox Oct 7, 2016 • edited Loading

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 Oct 7, 2016 • edited Loading

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

tadatuta commented Oct 7, 2016

Choose a reason for hiding this comment

a-x- Feb 15, 2017 • edited Loading

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 Oct 7, 2016 •

edited

Loading

qfox Oct 7, 2016 •

edited

Loading

qfox Oct 7, 2016 •

edited

Loading

a-x- Feb 15, 2017 •

edited

Loading