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.

Sign up for GitHub

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

Jump to bottom

[FEATURE] File tags component #141

Merged
merged 8 commits into from
Jun 23, 2016
Merged

[FEATURE] File tags component #141

merged 8 commits into from
Jun 23, 2016

Conversation

thatkookooguy
Copy link
Member

Change Summary

More info

new additional modes:

ABC:         ["abc"],
ADA:         ["ada|adb"],
C9Search:    ["c9search_results"],
Dockerfile:  ["^Dockerfile"],
Dummy:       ["dummy"],
DummySyntax: ["dummy"],
Eiffel:      ["e|ge"],
EJS:         ["ejs"],
Elixir:      ["ex|exs"],
Elm:         ["elm"],
Erlang:      ["erl|hrl"],
Fortran:     ["f|f90"],
Gcode:       ["gcode"],
Gitignore:   ["^.gitignore"],
Gobstones:   ["gbs"],
HTML_Elixir: ["eex|html.eex"],
Io:          ["io"],
Lean:        ["lean|hlean"],
Mask:        ["mask"],
Maze:        ["mz"],
NSIS:        ["nsi|nsh"],
Praat:       ["praat|praatscript|psc|proc"],
Razor:       ["cshtml"],
RST:         ["rst"],
Smarty:      ["smarty|tpl"],
SQLServer:   ["sqlserver"],
Swift:       ["swift"],
Tex:         ["tex"],
Vala:        ["vala"],
VHDL:        ["vhd|vhdl"],
Wollok:      ["wlk|wpgm|wtest"],
Django:      ["html"]

Before you submit a PR, make sure you did the following things:

  • did you link this PR to an issue?
  • did you lint your changes to both javascript and scss?
  • "I'm pretty sure I'll be able to read and understand this PR, even if I wasn't the author." - _said the PR author_

@thatkookooguy thatkookooguy self-assigned this Jun 19, 2016
@thatkookooguy thatkookooguy temporarily deployed to kibibit-demo-pr-141 June 19, 2016 23:30 Inactive
houndci-bot
houndci-bot reviewed Jun 19, 2016
View reviewed changes
app/models/fileService.js
var fileExtension = filepath.substring(filepath.lastIndexOf('.') + 1, filepath.length);
var mime;

switch(fileExtension) {
Copy link

Choose a reason for hiding this comment

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

One space required after "switch" keyword

@neilkalman-redkix
hound fixes
66cdb72 
good boy!
@thatkookooguy thatkookooguy temporarily deployed to kibibit-demo-pr-141 June 19, 2016 23:32 Inactive
houndci-bot
houndci-bot reviewed Jun 19, 2016
View reviewed changes
app/models/fileService.js
filepath.substring(filepath.lastIndexOf('.') + 1, filepath.length);
var mime;

switch(fileExtension) {
Copy link

Choose a reason for hiding this comment

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

One space required after "switch" keyword

@thatkookooguy thatkookooguy temporarily deployed to kibibit-demo-pr-141 June 19, 2016 23:37 Inactive
@thatkookooguy thatkookooguy temporarily deployed to kibibit-demo-pr-141 June 20, 2016 08:03 Inactive
@thatkookooguy thatkookooguy temporarily deployed to kibibit-demo-pr-141 June 20, 2016 15:07 Inactive
@ortichon
Copy link
Member

First of all, it's great that we support Django, I'll be able to use Kibibit as my new code editor for my next "real work" project 👍

ortichon
ortichon reviewed Jun 21, 2016
View reviewed changes
app/models/fileService.js
function extraTypes(filepath) {
if (filepath.indexOf('.') !== -1) {
var fileExtension =
filepath.substring(filepath.lastIndexOf('.') + 1, filepath.length);
Copy link
Member

@ortichon ortichon Jun 21, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

nit -> I would (you thought me) do something like:

var extensionStart = filepath.lastIndexOf('.') +1;
var fileExtension = filepath.substring(extensionStart, filepath.length);

but of-course it's ok as is 😺

ortichon
ortichon reviewed Jun 21, 2016
View reviewed changes
app/models/fileService.js
tags.pop();

tags.forEach(function(tag) {
switch (tag) {
Copy link
Member

Choose a reason for hiding this comment

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

💡 reminder -> add min 😄

@ortichon
Copy link
Member

ortichon commented Jun 21, 2016 •
edited
Loading

Well done, the comments I wrote are non-crucial, we can merge it even "as is", so I'll give it a
LGTM

Approved with PullApprove

@thatkookooguy
Copy link
Member Author

@dunaevsky , this is ready for you to do a CR :-)

@thatkookooguy thatkookooguy merged commit 71e6341 into master Jun 23, 2016
@thatkookooguy thatkookooguy deleted the feature-file-tags-component branch June 23, 2016 13:55
@dunaevsky
Copy link
Member

LGTM ;ׁׁ)

@dunaevsky
Copy link
Member

Did I do it right?

@thatkookooguy
Copy link
Member Author

@dunaevsky yup :-)

neilkalman-redkix pushed a commit that referenced this pull request Jul 4, 2016
@thatkookooguy
Merge pull request #141 from Kibibit/feature-file-tags-component
55d5605 
[FEATURE] File tags component
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@ortichon ortichon

@dunaevsky dunaevsky

Labels
Back End dependencies Front End New Feature
Projects
None yet
Milestone
01. Create a kickass **code editor** ...
Development

Successfully merging this pull request may close these issues.

update ace to get new improvements Create file tags component on server
5 participants
@thatkookooguy @ortichon @dunaevsky @houndci-bot @neilkalman-redkix