JoshCheek / atom-seeing-is-believing

Seeing is Believing integration for the Atom text editor.
Do What The F*ck You Want To Public License
62 stars 4 forks source link

Update README.md for snippet installation #26

Closed ckib16 closed 7 years ago

ckib16 commented 7 years ago

Help for newcomers on how to enable SiB snippets in Atom editor

This wasn't immediately obvious to me after watching the video and reading the docs.

Great work!

JoshCheek commented 7 years ago

Hi, thx thx ^_^

I mention some in the readme currently, can you see if those work? eg this one is s_arb Perhaps the language wasn't set to Ruby? Maybe I should just make them global since the probability of a namespace collision is negligible?

snippets

ckib16 commented 7 years ago

Hey @JoshCheek

Hmm. Just to be clear...

I did see you mention the snippets in current README

But...

So my PR changes were geared around installing the snippets to get them to work. In other words, getting noobs to the point where they could type s_arb and atom would auto-expand the snippet code.

Does that make sense?

Did your snippets automatically work for you after installing SiB atom package & gem with no other work required?

ckib16 commented 7 years ago

Ohhh...are you trying to show in the gif that AFTER you set the file type to .rb then Atom recognized s_arb as a snippet and allowed expansion?

That is something I did not try out. And you may be right.

I just know the snippets didn't expand for me and so I brute forced it by manually copy / pasting in as new snippet.

Let me check....

JoshCheek commented 7 years ago

Hmm, it's strange, I haven't experienced this issue before. The most likely thing I can think of is that the language wasn't set to Ruby, so it didn't match the snippets' scope. I'll see if I can find someone willing to try reproducing the issue.

ckib16 commented 7 years ago

Nope - you're right @JoshCheek

So maybe I'll just change PR to emphasize ensuring file type is set to .rb to ensure snippets work?

ckib16 commented 7 years ago

I updated README with results of our conversation above. Let me know what you think.

JoshCheek commented 7 years ago

Lets just remove the Ruby scope. It clearly trips people up, I've hit it before, too, if we scope it at something higher, then it removes that hurdle. I don't think we need to worry about collision due to their names being pretty unique. I'd also prefer to avoid encouraging users to save the file, I almost never have that use case, so it seems like it would be polluting their file system just to set the language (note that you can set the scope with Control-Shift-L or by clicking the current language in the lower right).

Would you be willing to do that? I don't know what the scope should be, I'd prob figure it out by looking for a snippet that is language agnostic and seeing what they used there, if I couldn't find one, probably just try .editor since it just seems probable, in that case, use Command-Option-I to pull up dev tools and inspect the DOM to make sure it's a reasonable guess. Might also say in the docs, eg in the snippets or keybindings sections.

JoshCheek commented 7 years ago

These two might have info on a better selector:

ckib16 commented 7 years ago

Ah I gotcha.

Honestly - that stuff is above my level. I understand conceptually what you are saying, but you would be a much better judge.

My take - asking people to set file extension to .rb is no big deal at all. I'd say lean that way, but I understand if you want to solve something bigger.

Feel free to toss this PR, and thanks again for making this! Spreading the word to my co-workers.

JoshCheek commented 7 years ago

I can do it if you like, just didn't want to take the opportunity from you :) It should be a change to this line, which is scoping the snippets so that they are only available inside of ruby code. If we change it to something else, like .source.python (I assume), they would only be available within python code. So it's a question of what's the right thing to replace it with.

JoshCheek commented 7 years ago

I went ahead and updated the scope.

Thanks for bringing this to my attention, I've had a number of UX issues over the years, but I do think it is incredibly important, so it's good that we're finding and fixing things like this ^_^

ckib16 commented 7 years ago

Ah sorry @JoshCheek - got caught up in work yesterday :)

Yes - your change makes more sense when you pointed the line out. That should work well.

Thanks for the help!