On Fri, Jun 10, 2016 at 8:30 AM, Rory McGuire <rjmcgu...@gmail.com> wrote: > On Thu, Jun 9, 2016 at 10:48 PM, Steven Schveighoffer via > Digitalmars-d-announce <digitalmars-d-announce@puremagic.com> wrote: >> On 6/9/16 4:37 PM, Sönke Ludwig wrote: >>> >>> Am 09.06.2016 um 15:06 schrieb Steven Schveighoffer: >>>> >>>> On 6/8/16 2:45 PM, Sönke Ludwig wrote: >>>> (...) >>>>> >>>>> Apart from what I've already mentioned in my first reply to Jacob, I >>>>> think there is nothing else that couldn't be solved in either case. >>>> >>>> >>>> "It's still possible to put something else in front of it" >>>> >>>> I didn't get this. How is /+ different from /*? I thought the only issue >>>> was the nesting. >>> >>> >>> I mean together with the restriction that it has to be the first /+ +/ >>> comment, it is possible to put // and /* style comments in front of it >>> without interference. >> >> >> Oh, didn't see that aspect. I'll answer this with your last point. >> >>> >>>>> Okay, so at least 3 people favor supporting other comment styles, while >>>>> I'm not convinced that supporting all comment styles is necessarily >>>>> better, I wouldn't mind pulling an update. The relevant code section is >>>>> here: >>>>> >>>>> https://github.com/dlang/dub/blob/b02c18991b0cb4627eb0c943efd6ca3e69192751/source/dub/dub.d#L288 >>>>> >>>>> >>>> >>>> Thanks. Perhaps more debate is fruitless until someone steps up with an >>>> implementation :) >>> >>> >>> Rory has started an implementation: https://github.com/dlang/dub/pull/872 >>> >>> But this has brought up another issue. The idea was to allow the recipe >>> comment to be anywhere in the file and be of any comment style. However, >>> that basically almost requires a full D lexer (at least string literals >>> need to be parsed in addition to the comment styles). >>> >>> I'm reluctant to include this in the current state, because the results >>> for a program such as the following will be very confusing: >>> >>> bool foo(string str) >>> { >>> return str.startsWith("/*"); >>> } >>> >>> /* dub.sdl: ... */ >>> void main() >>> { >>> } >>> >>> The string literal will be recognized as the start of a comment and thus >>> the real comment below will not be recognized. So I think we should >>> either have a generic and correct version, or one that is restricted >>> similar to the current one to sidestep such issues. >> >> >> I think the idea to include it anywhere in the file is not worth the >> trouble. I also wouldn't want to scroll through an entire file for such a >> thing. >> >> But it also brings up the point, what if the first /+ comes after: >> >> return str.startsWith("/+"); >> >>> >>>>> 1.0.0-rc.1 is scheduled for Monday morning, so it should ready by then >>>>> to avoid stretching the release schedule (it's technically a breaking >>>>> change!). >>>> >>>> >>>> How would this be a breaking change? It seems an additive feature, and >>>> I'm not sure it's required to be there before the first 1.0 release. >>> >>> >>> There could be a /* */ comment before the /+ dub.*: +/ one, which is now >>> not considered as a recipe comment candidate. Depending on its contents, >>> the behavior could change once /* */ is also recognized. However, that >>> case would be easily detectable and a warning could be emitted instead, >>> while falling back to the old behavior. So it's not really an issue >>> after all. >>> >> >> Yeah, comments are not hard to parse, if they are the first thing you are >> expecting. >> >> I would say it doesn't have to be the first comment, but it has to be one of >> the first things in the file. You can simply throw away other comments. >> >> You may want to just tell the user: look, this isn't perfect, it's not a D >> parser. Expect reasonable results for reasonable situations :) If you have a >> line of code that looks like it could match the sdl file, then put the true >> sdl file above it. >> >> -Steve > > > > Could we not just make it a requirement that if the file starts with > #!... on the first line then the second line _must_ be a comment with > the dub file definition? > > It will be frustrating to try find the dub definition if its not near > the top and if the definition starts to be complicated so that you > can't see the copyright or whatever would usually be in the first > comment then you probably shouldn't be using a single file dub project > anyway. > > If that is released with dub 1.0 then the restriction can always be > loosened up if there is enough demand.
I made a version that ignores comment like characters in strings. I've also made a version that requires the recipe to be on the second line. Both are in my fork of dub. I can fix my pull request to which ever one you guys prefer. The one that handles recipe anywhere has a flaw where it will still recognize a dub recipe even if the recipe is inside a nested comment (one would expect that to be commented out. The one that handles recipe on second line only passes the following unittest: unittest { // first line must be #! assertThrown!Exception(SingleFilePackage("/* dub.json: {} */void main(){}")); // missing dub recipe assertThrown!Exception(SingleFilePackage("#!/asdf\n/* dub: {} */void main(){}")); // single file package must contain code assertThrown!Exception(SingleFilePackage("#!/asdf\n/* dub.sdl: */")); // documentation style comments not supported for dub recipe comment assertThrown!Exception(SingleFilePackage("#!/asdf\n/** dub.json: {} */void main(){}")); // documentation style comments not supported for dub recipe comment assertThrown!Exception(SingleFilePackage("#!/asdf\n/++ dub.json: {} +/void main(){}")); // unclosed dub recipe comment assertThrown!Exception(SingleFilePackage("#!/asdf\n/* dub.json: {} * /void main(){}")); assertNotThrown!Exception(SingleFilePackage("#!/asdf\n/* dub.sdl: {name:\"*\"} */void main(){}")); assertNotThrown!Exception(SingleFilePackage("#!/asdf\n/+ dub.json: /* some sdl comment */ +/void main(){}")); assertNotThrown!Exception(SingleFilePackage("#!/asdf\n/* dub.json: {} */void main(){}")); assertNotThrown!Exception(SingleFilePackage("#!/asdf\n/* dub.sdl: {} */void main(){}")); }