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.

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

Better handle markdown headings when editing issues #334

Open
jwkicklighter opened this issue Sep 20, 2019 · 15 comments
Open

Better handle markdown headings when editing issues #334

jwkicklighter opened this issue Sep 20, 2019 · 15 comments

Comments

@jwkicklighter
Copy link

#197 took care of being able to use # at the start of a line, but I just noticed that there isn't a great user flow for editing an issue that already has markdown headings.

  1. Create an issue with at least 1 heading
  2. Edit the issue with lab issue edit [ID]
  3. Make changes and save
  4. The issue will now have its heading(s) stripped out

I'm not sure what the best course would be, but perhaps requiring escaped hashes wasn't the best solution for #197.

@zaquestion
Copy link
Owner

Haha, yeah I've been thinking about this a lot myself lately, interesting to see you bringing it back up. I'm really not sure what the best way to differentiate markdown from git comments when the default comment char # is used.

Personally I have no interest in changing the comment char in git myself, so I'd like to find a solution that just works.

@jwkicklighter
Copy link
Author

It's definitely an interesting problem since it's not likely that people mix these two formats often. I'll have a look around and see if I can think of anything.

@zaquestion
Copy link
Owner

I think in an edit context, I'm loading text that won't have comments. So perhaps could escape them all on load. Thoughts?

... Tired -- not sure if making sense

@claytonrcarter
Copy link
Collaborator

claytonrcarter commented Sep 24, 2019 via email

@jwkicklighter
Copy link
Author

I was going to suggest something similar, but escaping them on application exit instead. That way you could edit as if it's normal markdown and lab would just handle things for you.

@zaquestion
Copy link
Owner

zaquestion commented Oct 25, 2019

hub seems to have adapted their approach just noticed this on my latest pr to this repo. Perhaps something like this could work well for us.
@claytonrcarter @jwkicklighter thoughts?

fix #314: --version not showing lab version
# ------------------------ >8 ------------------------
# Do not modify or remove the line above.
# Everything below it will be ignored.

Requesting a pull to zaquestion:master from zaquestion:fix_version_flag

Write a message for this pull request. The first block
of text is the title and the rest is the description.

@jwkicklighter
Copy link
Author

I'm not sure how this fixes the problem of supporting markdown headers that start with # characters, but I could be missing something since I don't use hub myself.

@zaquestion
Copy link
Owner

https://github.com/github/hub/blob/12083a59317dc879f26e2464974f175d4fd0a85b/github/editor.go#L86-L93 They read the content literally until the "scissors" line and then discard everything below it. So lines that begin with # above the scissors line aren't removed.

# ------------------------ >8 ------------------------

@jwkicklighter
Copy link
Author

Oh, that makes sense! That seems like a good solution, but the scissors line probably doesn't even matter unless lab was to include some instructions underneath it. If everything below that line is ignored, then the only reason for including it is to include some sort of prefilled message to the user. lab could just interpret things literally, the way that hub handles whatever is above the line. But like I said, it would probably be good to include instructions anyway.

@zaquestion
Copy link
Owner

In an edit context it probably doesn't make as much sense, because yeah, there might now be in other information to show. The cutline would mostly be useful for the create flows which include instructions at the bottom and a summary of commits when creating merge request.

@jwkicklighter
Copy link
Author

I think the biggest thing I got from the hub code is that it isn't relying on the git mechanism for editing the MR/Issue text. It just opens up an editor window and waits for it to be closed. Is there a reason that lab does this differently?

(Sorry if my vagueness is hard to follow and/or just totally incorrect... I still only partially understand what lab is doing under-the-hood since I'm not a golang dev and haven't spent the time to fully grep through the codebase.)

@zaquestion
Copy link
Owner

@jwkicklighter lab essentially operates the same way, in fact the implementation in lab was inspired by hubs own implementation. It looks like they've moved ahead and we're still like this. In large tho, we open the editor and wait for it to close. We store the file being edited within .git/, but I believe hub does the same (or at least the used to). At this point I think adjusting our implementation to use a cutline is the right way to go as it address this issue updates our behavior to be similar to hub

@jwkicklighter
Copy link
Author

@zaquestion Let me know if there's any way I can help, since I keep bringing up these issues 🙂

@zaquestion
Copy link
Owner

It's pretty much just a matter of swapping in the hub logic, which shouldn't be too bad. LMK if you wanna try and take on implementation, I'm also talking with a potential new contributor and pointed them at this, but I'm not sure yet if they'll take it on, they might jump in somewhere else. So feel free to tackle it, it'll happen soon enough for sure.

hub impl: https://github.com/github/hub/blob/12083a59317dc879f26e2464974f175d4fd0a85b/github/editor.go#L75-L100

lab impl: https://github.com/zaquestion/lab/blob/master/internal/git/edit.go#L94-L107

And then adding the scizzors line into the edit templates. Below is the mr create one, but may need to get issue and snippet, but I think thats everything. (also fix tests)
https://github.com/zaquestion/lab/blob/master/cmd/mr_create.go#L195-L203

@asdf8601
Copy link

asdf8601 commented Mar 10, 2020

+1, the same happens to me 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants